From d57edd90b2090afbef7a00af64e9d5b06457f99a Mon Sep 17 00:00:00 2001 From: Norman Rusch Date: Fri, 27 Sep 2024 10:34:10 +0200 Subject: [PATCH] Fix eslint errors and warnings after migration --- src/carousel.test.ts | 21 ++++++++------------- src/carousel.ts | 19 +++---------------- src/features/buttons/index.test.ts | 6 ++---- src/features/buttons/index.ts | 12 ------------ src/features/mouse/index.test.ts | 8 ++++---- src/features/mouse/index.ts | 22 ++++++++-------------- src/features/pagination/index.test.ts | 6 ++---- src/features/pagination/index.ts | 8 -------- src/proxy.test.ts | 2 -- src/utils/cache.ts | 3 --- 10 files changed, 27 insertions(+), 80 deletions(-) diff --git a/src/carousel.test.ts b/src/carousel.test.ts index 96768a76..f1e88d04 100644 --- a/src/carousel.test.ts +++ b/src/carousel.test.ts @@ -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.')); }); @@ -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(); @@ -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]); }); @@ -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]); }); @@ -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]); }); @@ -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]); }); diff --git a/src/carousel.ts b/src/carousel.ts index 9da7b537..95ea04b5 100644 --- a/src/carousel.ts +++ b/src/carousel.ts @@ -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 */ } /** @@ -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(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); diff --git a/src/features/buttons/index.test.ts b/src/features/buttons/index.test.ts index 5366e6ff..ffb3fbbc 100644 --- a/src/features/buttons/index.test.ts +++ b/src/features/buttons/index.test.ts @@ -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 = jest.fn(() => null); new Carousel(el, { @@ -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 = jest.fn(() => undefined); new Carousel(el, { diff --git a/src/features/buttons/index.ts b/src/features/buttons/index.ts index a39c2f94..b69dfd78 100644 --- a/src/features/buttons/index.ts +++ b/src/features/buttons/index.ts @@ -201,9 +201,6 @@ 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, }, { @@ -211,9 +208,6 @@ export class Buttons implements IFeature { 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, }, ]; @@ -252,13 +246,7 @@ export class Buttons implements IFeature { const buttons = fromCache(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); }); diff --git a/src/features/mouse/index.test.ts b/src/features/mouse/index.test.ts index 60cfd896..41908d24 100644 --- a/src/features/mouse/index.test.ts +++ b/src/features/mouse/index.test.ts @@ -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', () => { @@ -164,9 +164,9 @@ describe('Mouse feature', () => { }); it('should call hooks', () => { - const onStart = jest.fn(); - const onDrag = jest.fn(); - const onEnd = jest.fn(); + const onStart = jest.fn(); + const onDrag = jest.fn(); + const onEnd = jest.fn(); document.body.innerHTML = fixture(4); const el = querySelector('.caroucssel'); diff --git a/src/features/mouse/index.ts b/src/features/mouse/index.ts index c1dee1f4..8c0ab4cb 100644 --- a/src/features/mouse/index.ts +++ b/src/features/mouse/index.ts @@ -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; }; @@ -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 @@ -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 }); } @@ -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 }); @@ -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 }); diff --git a/src/features/pagination/index.test.ts b/src/features/pagination/index.test.ts index f9025f5e..0dc93ed2 100644 --- a/src/features/pagination/index.test.ts +++ b/src/features/pagination/index.test.ts @@ -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 = jest.fn(() => null); new Carousel(el, { @@ -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 = jest.fn(() => undefined); new Carousel(el, { diff --git a/src/features/pagination/index.ts b/src/features/pagination/index.ts index 949e4db6..c927b407 100644 --- a/src/features/pagination/index.ts +++ b/src/features/pagination/index.ts @@ -84,8 +84,6 @@ export type Options = { type Configuration = Required; const DEFAULTS: Configuration = { - // @TODO: ESLint don't like the nested template literals and loops. - /* eslint-disable indent */ template: ({ className, controls, pages, label, title }: Context) => `
    ${pages.map((page, index) => { @@ -100,7 +98,6 @@ const DEFAULTS: Configuration = { }).join('')}
`, - /* eslint-enable indent */ className: 'pagination', label: ({ index }) => `${index + 1}`, @@ -197,8 +194,6 @@ export class Pagination implements IFeature { const buttons = Array.from(pagination.querySelectorAll('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; }); @@ -231,9 +226,6 @@ export class Pagination implements IFeature { const buttons = fromCache(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); }); diff --git a/src/proxy.test.ts b/src/proxy.test.ts index 7b55af9e..6c2d7b54 100644 --- a/src/proxy.test.ts +++ b/src/proxy.test.ts @@ -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 */ }); }); diff --git a/src/utils/cache.ts b/src/utils/cache.ts index 9bb00283..374240a7 100644 --- a/src/utils/cache.ts +++ b/src/utils/cache.ts @@ -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;