Skip to content

Commit

Permalink
Fix eslint errors and warnings after migration
Browse files Browse the repository at this point in the history
  • Loading branch information
schorfES committed Sep 27, 2024
1 parent 25a5423 commit 73e39a5
Show file tree
Hide file tree
Showing 10 changed files with 27 additions and 80 deletions.
21 changes: 8 additions & 13 deletions src/carousel.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,13 +40,13 @@ describe('Carousel', () => {

it('should throw an error when not passing an element', () => {
// Test untyped behaviour of constructor
// @ts-ignore
// @ts-expect-error test invalid parameters of constructor
expect(() => new Carousel())
.toThrow(new Error('Carousel needs a dom element but "undefined" was passed.'));
// @ts-ignore
// @ts-expect-error test invalid parameters of constructor
expect(() => new Carousel(true))
.toThrow(new Error('Carousel needs a dom element but "boolean" was passed.'));
// @ts-ignore
// @ts-expect-error test invalid parameters of constructor
expect(() => new Carousel({}))
.toThrow(new Error('Carousel needs a dom element but "object" was passed.'));
});
Expand Down Expand Up @@ -214,8 +214,7 @@ describe('Carousel', () => {

const scrollTo = jest.spyOn(el, 'scrollTo');

// Test untyped behaviour of constructor
// @ts-ignore
// @ts-expect-error test untyped behaviour of constructor
const carousel = new Carousel(el, { index: [] });
expect(carousel.index).toEqual([0]);
expect(scrollTo).not.toHaveBeenCalled();
Expand Down Expand Up @@ -312,8 +311,7 @@ describe('Carousel', () => {

const carousel = new Carousel(el);

// Test untyped behaviour of setter
// @ts-ignore
// @ts-expect-error test untyped behaviour of setter
carousel.index = [];
expect(carousel.index).toEqual([0, 1]);
});
Expand All @@ -326,8 +324,7 @@ describe('Carousel', () => {

const carousel = new Carousel(el);

// Test untyped behaviour of setter
// @ts-ignore
// @ts-expect-error test untyped behaviour of setter
carousel.index = 2;
expect(carousel.index).toEqual([0, 1]);
});
Expand All @@ -340,8 +337,7 @@ describe('Carousel', () => {

const carousel = new Carousel(el);

// Test untyped behaviour of setter
// @ts-ignore
// @ts-expect-error test untyped behaviour of setter
carousel.index = undefined;
expect(carousel.index).toEqual([0, 1]);
});
Expand All @@ -354,8 +350,7 @@ describe('Carousel', () => {

const carousel = new Carousel(el);

// Test untyped behaviour of setter
// @ts-ignore
// @ts-expect-error test untyped behaviour of setter
carousel.index = null;
expect(carousel.index).toEqual([0, 1]);
});
Expand Down
19 changes: 3 additions & 16 deletions src/carousel.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,18 +152,10 @@ export class Carousel implements ICarousel {
// testing the behavior in several browsers and are considered at "best fit"
// without visible side effects to the UI. The value for the "scroll" event
// correlates with the timing of scroll-behaviour: smooth.
//
// We disable @typescript-eslint/unbound-method here because we already bound
// the functions while creating a debounced version. This would also cause
// reference errors when tying to access these function references when used
// with removeEventListeners() (see: destroy())
//
/* eslint-disable @typescript-eslint/unbound-method */
this._onScroll = debounce(this._onScroll.bind(this), 45);
this._onResize = debounce(this._onResize.bind(this), 25);
el.addEventListener(EVENT_SCROLL, this._onScroll);
window.addEventListener(EVENT_RESIZE, this._onResize);
/* eslint-enable @typescript-eslint/unbound-method */
}

/**
Expand Down Expand Up @@ -440,22 +432,17 @@ export class Carousel implements ICarousel {
const { el } = this;

// Remove created id if it was created by carousel:
ID_MATCH.test(el.id) && el.removeAttribute('id');
if(ID_MATCH.test(el.id)) {
el.removeAttribute('id');
}

// Destroy attached features:
const features = fromCache<IFeature[]>(this, CACHE_KEY_FEATURES) as IFeature[];
features.forEach((feature) => feature.destroy());

// Remove events:
//
// We need to work the the function reference. Using .bind() would create a
// new referenced instance of the callback function. We already created a
// bound version of these function within the constructor.
//
/* eslint-disable @typescript-eslint/unbound-method */
el.removeEventListener(EVENT_SCROLL, this._onScroll);
window.removeEventListener(EVENT_RESIZE, this._onResize);
/* eslint-enable @typescript-eslint/unbound-method */

// Clear cache:
clearFullCache(this);
Expand Down
6 changes: 2 additions & 4 deletions src/features/buttons/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,8 +114,7 @@ describe('Buttons feature', () => {
document.body.innerHTML = fixture(3);
const el = querySelector('.caroucssel');

// Test when js custom implementation returns null
// @ts-ignore
// @ts-expect-error test when js custom implementation returns null
const template: jest.Mock<string, []> = jest.fn(() => null);

new Carousel(el, {
Expand All @@ -130,8 +129,7 @@ describe('Buttons feature', () => {
document.body.innerHTML = fixture(3);
const el = querySelector('.caroucssel');

// Test when js custom implementation returns undefined
// @ts-ignore
// @ts-expect-error test when js custom implementation returns undefined
const template: jest.Mock<string, [Context]> = jest.fn(() => undefined);

new Carousel(el, {
Expand Down
12 changes: 0 additions & 12 deletions src/features/buttons/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -201,19 +201,13 @@ export class Buttons implements IFeature {
label: nextLabel,
title: nextTitle,
className: [className, nextClassName].join(' '),
// The onClick listener is already bound in the constructor.
//
// eslint-disable-next-line @typescript-eslint/unbound-method
handler: this._onNext,
},
{
controls: el.id,
label: previousLabel,
title: previousTitle,
className: [className, previousClassName].join(' '),
// The onClick listener is already bound in the constructor.
//
// eslint-disable-next-line @typescript-eslint/unbound-method
handler: this._onPrev,
},
];
Expand Down Expand Up @@ -252,13 +246,7 @@ export class Buttons implements IFeature {
const buttons = fromCache<Button[]>(this, CACHE_KEY_BUTTONS);

buttons?.forEach((button): void => {
// The onClick listener is already bound in the constructor.
//
// eslint-disable-next-line @typescript-eslint/unbound-method
button?.removeEventListener(EVENT_CLICK, this._onPrev);
// The onClick listener is already bound in the constructor.
//
// eslint-disable-next-line @typescript-eslint/unbound-method
button?.removeEventListener(EVENT_CLICK, this._onNext);
button?.parentNode?.removeChild(button);
});
Expand Down
8 changes: 4 additions & 4 deletions src/features/mouse/index.test.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { fixture, querySelector, triggerMouse } from '../../__setup__/helpers';
import { Carousel } from '../../carousel';

import { HookEvent, Mouse } from './index';
import { HookEvent, HookEventHandler, Mouse } from './index';


describe('Mouse feature', () => {
Expand Down Expand Up @@ -164,9 +164,9 @@ describe('Mouse feature', () => {
});

it('should call hooks', () => {
const onStart = jest.fn<void, [HookEvent]>();
const onDrag = jest.fn<void, [HookEvent]>();
const onEnd = jest.fn<void, [HookEvent]>();
const onStart = jest.fn<HookEventHandler, [HookEvent]>();
const onDrag = jest.fn<HookEventHandler, [HookEvent]>();
const onEnd = jest.fn<HookEventHandler, [HookEvent]>();

document.body.innerHTML = fixture(4);
const el = querySelector('.caroucssel');
Expand Down
22 changes: 8 additions & 14 deletions src/features/mouse/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,17 +53,17 @@ export type Options = {
/**
* A hook function that is called when the user stats to drag.
*/
onStart?: ((event: HookEvent) => void);
onStart?: HookEventHandler;

/**
* A hook function that is called when the user is dragging.
*/
onDrag?: ((event: HookEvent) => void);
onDrag?: HookEventHandler;

/**
* A hook function that is called when the user stops to drag.
*/
onEnd?: ((event: HookEvent) => void);
onEnd?: HookEventHandler;

};

Expand All @@ -74,6 +74,11 @@ export type HookEvent = {
originalEvent: Event,
};

/**
* The shape of an event hander function.
*/
export type HookEventHandler = (event: HookEvent) => void;

/**
* The keys in the options that are hooks.
* @internal
Expand Down Expand Up @@ -131,10 +136,6 @@ export class Mouse implements IFeature {
const { el } = proxy;
const element = el as HTMLElement;
element.style.cursor = config.indicator ? CURSOR_GRAB : '';

// The handler is already bound in the constructor.
//
// eslint-disable-next-line @typescript-eslint/unbound-method
el.addEventListener(EVENT_START, this._onStart, { passive: true });
}

Expand Down Expand Up @@ -181,12 +182,8 @@ export class Mouse implements IFeature {
element.style.scrollSnapType = 'none';
element.style.cursor = config.indicator ? CURSOR_GRABBING : '';

// The handlers are already bound in the constructor.
//
/* eslint-disable @typescript-eslint/unbound-method */
window.addEventListener(EVENT_DRAG, this._onDrag, { passive: true });
window.addEventListener(EVENT_END, this._onEnd, { passive: true });
/* eslint-enable @typescript-eslint/unbound-method */

// Call the hook:
config.onStart?.({ originalEvent: event });
Expand Down Expand Up @@ -261,11 +258,8 @@ export class Mouse implements IFeature {
writeCache(this, CACHE_KEY_TIMEOUT, timeout);

// The handlers are already bound in the constructor.
//
/* eslint-disable @typescript-eslint/unbound-method */
window.removeEventListener(EVENT_DRAG, this._onDrag);
window.removeEventListener(EVENT_END, this._onEnd);
/* eslint-enable @typescript-eslint/unbound-method */

// Call the hook:
config.onEnd?.({ originalEvent: event });
Expand Down
6 changes: 2 additions & 4 deletions src/features/pagination/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -169,8 +169,7 @@ describe('Pagination feature', () => {
document.body.innerHTML = fixture(3);
const el = querySelector('.caroucssel');

// Test when js custom implementation returns null
// @ts-ignore
// @ts-expect-error test when js custom implementation returns null
const template: jest.Mock<string, []> = jest.fn(() => null);

new Carousel(el, {
Expand All @@ -185,8 +184,7 @@ describe('Pagination feature', () => {
document.body.innerHTML = fixture(3);
const el = querySelector('.caroucssel');

// Test when js custom implementation returns undefined
// @ts-ignore
// @ts-expect-error test when js custom implementation returns undefined
const template: jest.Mock<string, []> = jest.fn(() => undefined);

new Carousel(el, {
Expand Down
8 changes: 0 additions & 8 deletions src/features/pagination/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,6 @@ export type Options = {
type Configuration = Required<Options>;

const DEFAULTS: Configuration = {
// @TODO: ESLint don't like the nested template literals and loops.
/* eslint-disable indent */
template: ({ className, controls, pages, label, title }: Context) => `
<ul class="${className}">
${pages.map((page, index) => {
Expand All @@ -100,7 +98,6 @@ const DEFAULTS: Configuration = {
}).join('')}
</ul>
`,
/* eslint-enable indent */

className: 'pagination',
label: ({ index }) => `${index + 1}`,
Expand Down Expand Up @@ -197,8 +194,6 @@ export class Pagination implements IFeature {
const buttons = Array.from(pagination.querySelectorAll<HTMLButtonElement>('button'))
.map((button) => {
// The onClick listener is already bound in the constructor.
//
// eslint-disable-next-line @typescript-eslint/unbound-method
button.addEventListener('click', this._onClick, true);
return button;
});
Expand Down Expand Up @@ -231,9 +226,6 @@ export class Pagination implements IFeature {
const buttons = fromCache<HTMLButtonElement[]>(this, CACHE_KEY_BUTTONS);

buttons?.forEach((button) => {
// The onClick listener is already bound in the constructor.
//
// eslint-disable-next-line @typescript-eslint/unbound-method
button.removeEventListener('click', this._onClick);
button.parentNode?.removeChild(button);
});
Expand Down
2 changes: 0 additions & 2 deletions src/proxy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,13 +79,11 @@ describe('proxy', () => {
const proxy = new Proxy(carousel, [feat1, feat2, feat3]);
proxy.update(feat2);

/* eslint-disable @typescript-eslint/unbound-method */
expect(feat1.update).toHaveBeenCalledTimes(1);
expect(feat1.update).toHaveBeenCalledWith({ type: 'feature' });
expect(feat2.update).not.toHaveBeenCalled();
expect(feat3.update).toHaveBeenCalledTimes(1);
expect(feat3.update).toHaveBeenCalledWith({ type: 'feature' });
/* eslint-enable @typescript-eslint/unbound-method */
});

});
3 changes: 0 additions & 3 deletions src/utils/cache.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,3 @@
// The key of a WeakMap must be an object. There is no other type that matches
// or is valid for @typescript-eslint.
// eslint-disable-next-line @typescript-eslint/ban-types
type Reference = object;

type Storage = Map<string, unknown>;
Expand Down

0 comments on commit 73e39a5

Please sign in to comment.