davidjerleke / embla-carousel

A lightweight carousel library with fluid motion and great swipe precision.
https://www.embla-carousel.com
MIT License
5.37k stars 164 forks source link

[Bug]: any type for `Autoplay` plugin options #887

Closed musjj closed 3 weeks ago

musjj commented 1 month ago

Which variants of Embla Carousel are you using?

Steps to reproduce

import Autoplay from "embla-carousel-autoplay";

Autoplay({|})

When hovered:

(alias) Autoplay(userOptions?: any): CreatePluginType<{
                               ^^^
    play: (jump?: boolean | undefined) => void;
    stop: () => void;
    reset: () => void;
    isPlaying: () => boolean;
}, CreateOptionsType<{
...

The typescript definition itself seems fine:

declare function Autoplay(userOptions?: AutoplayOptionsType): AutoplayType;

Weirdly the completion works in codesandbox, but not in my local VS Code environment.

Expected Behavior

Types should not be any.

Additional Context

Not sure of any ways of fixing it. But it does look suspiciously similar to this years-old resolved bug: https://github.com/microsoft/TypeScript/issues/7485

What browsers are you seeing the problem on?

All browsers

Version

v8.1.3

CodeSandbox

https://codesandbox.io/p/github/musjj/embla-any-repro

Before submitting

davidjerleke commented 3 weeks ago

Hi @musjj,

Thanks for your bug report. Please create a temporary repository and add instructions on how to reproduce this problem here. The StackBlitz you shared isn't enough for me to reproduce it.

Let me know if you intend to provide a repository and steps to reproduce the problem.

musjj commented 3 weeks ago

The codesandbox I linked is just a hosted GitHub repo: https://github.com/musjj/embla-any-repro

davidjerleke commented 3 weeks ago

@musjj thanks for pointing that out,

You don't have a tsconfig.json so I guess TypeScript doesn't know how to resolve the module types. Adding this solves the problem:

tsconfig.json

{
  "compilerOptions": {
    "moduleResolution": "node"
  },
  "include": ["index.ts"],
  "exclude": ["node_modules"]
}
musjj commented 3 weeks ago

In most React-based projects, you would set moduleResolution to Bundler. With this config:

{
  "compilerOptions": {
    "module": "ESNext",
    "moduleResolution": "Bundler"
  },
  "include": ["index.ts"],
  "exclude": ["node_modules"]
}

The completion fails.

davidjerleke commented 3 weeks ago

In most React-based projects, you would set moduleResolution to Bundler.

@musjj I don’t know how widely the bundler setting is used but just claiming that without proof isn’t convincing.

Anyway, regardless if that’s true or not, Embla was around before the bundler setting was introduced so this is not a bug but a feature request. There’s no point of a duplicate issue because this is already tracked here:

But maybe you didn’t know that the above issue was related to that. So thanks if your intention was to help. Closing as a duplicate.

AntoLepejian commented 2 weeks ago

Hi @davidjerleke !

Thanks for actioning this! While this did resolve the issue, there appears to be some type incompatibility between the main package, and this plugin (maybe others too, haven't checked).

    const autoplayPlugin = Autoplay({
      playOnInit: false,
      delay: AUTOPLAY_DELAY_MS,
    });
    const [carouselRef, controller] = useEmblaCarousel(
      {
        ...opts,
      },
      [autoplayPlugin],
    );
Type 'AutoplayType' is not assignable to type 'CreatePluginType<LoosePluginType, {}>'.

Though it appears to be a type-only error, as the plugin itself works fine! (you'd have to suppress the TS error tho).

There's also a potentially related TS issue when accessing the plugin.

Screenshot 2024-06-12 at 9 04 00 PM

Versions: embla-carousel-autoplay": "^8.1.4 embla-carousel-react": "^8.1.4"

davidjerleke commented 2 weeks ago

@AntoLepejian does this happen if you pass the plugin directly too like below?

const [carouselRef, controller] = useEmblaCarousel(
  { ...opts },
  [Autoplay({
    playOnInit: false,
    delay: AUTOPLAY_DELAY_MS,
  })],
);

If not, I've found some errors that I have actioned in this PR:

AntoLepejian commented 2 weeks ago

@davidjerleke yeah happens regardless.

Here's my workaround

type UseCarouselParameters = Parameters<typeof useEmblaCarousel>;
type CarouselPlugin = UseCarouselParameters[1];

Autoplay({
          playOnInit: false,
          delay: AUTOPLAY_DELAY_MS,
        }) as unknown as NonUndefined<CarouselPlugin>[0]
davidjerleke commented 2 weeks ago

@AntoLepejian thanks. Let's see if this PR will resolve the issue then:

davidjerleke commented 2 weeks ago

@AntoLepejian feel free to try v8.1.5 out and see if it solves the problem.

AntoLepejian commented 2 weeks ago

@AntoLepejian feel free to try v8.1.5 out and see if it solves the problem.

@davidjerleke solved both issues 🚀 🚀 everything compiles beautifully without any TS hackery

davidjerleke commented 2 weeks ago

@AntoLepejian thanks for testing 👍!