xiel / embla-carousel-wheel-gestures

wheel interactions for Embla Carousel
https://embla-carousel-wheel-gestures.xiel.dev/react.html
MIT License
54 stars 8 forks source link

Adapt for the new embla-carousel v7 #170

Closed dermotduffy closed 2 years ago

dermotduffy commented 2 years ago
vercel[bot] commented 2 years ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
embla-carousel-wheel-gestures ✅ Ready (Inspect) Visit Preview Jul 6, 2022 at 11:28AM (UTC)
xiel commented 2 years ago

Hey @dermotduffy

thanks for opening the PR! Looks good to me 👍

Will try to merge this soon.

davidjerleke commented 2 years ago

Hi guys!

As you may have notices the lint command is failing on this branch. It's because of the plugin type imports that are used to define the plugin map here. I want the Embla core library to to include the plugin types (without the need to npm install all plugins) because the core library exposes the plugin API:s from its core API in v7 like this.

Any ideas how to achieve this with rollup?

Basically this is about how to include an external library installed in the node_modules directory in my end bundle. I haven't found something useful in the rollup documentation.

Feature description

I want to achieve the following:

My code making use of an external component: src/index.ts

import { Plugin1ApiType } from 'plugin1';
import { Plugin2ApiType } from 'plugin2';

type PluginType = Plugin1ApiType | Plugin2ApiType;

export type CoreLibraryApiType = {
  plugins: {
    plugin1?: Plugin1ApiType,
    plugin2?: Plugin2ApiType,
  }
};

export function CoreLibrary(plugins: PluginType[]): CoreLibraryApiType {
  const pluginApis = plugins.reduce((pluginApis, pluginApi) => {
    return { ...pluginApis, [plugin.name]: pluginApi };
  }, {})

  return { plugins: pluginApis };
};

Desired output: index.d.ts

declare type Plugin1ApiType = { ... }

declare type Plugin2ApiType = { ... }

export declare type CoreLibraryApiType = {
  plugins: {
    plugin1?: Plugin1ApiType,
    plugin2?: Plugin2ApiType,
  }
};

Usage:

import CoreLibrary from 'core-library';
import { Plugin1 } from 'plugin1';

const plugins = [Plugin1()];
const coreLibrary = CoreLibrary(plugins);

coreLibrary.plugins.plugin1?.someMethod();

Any help appreciated.

Best, David

xiel commented 2 years ago

I fixed the lint/type-check step for now using skipLibCheck https://github.com/xiel/embla-carousel-wheel-gestures/commit/1a4efa5ada48bdf8180e2f331147eccdb23c7a6c This option is used by many people anyway I think.

But yes, would be better if the type check would pass also with lib check active. Mhh... 🤔 I will think about it.

xiel commented 2 years ago

@davidjerleke I wonder if it would be better to not use these external type dependecies at all for the plugins. Instead use typescript to infer the plugin types from the passed plugins array.

API could look like this then:

import CoreLibrary from 'core-library';
import { Plugin1 } from 'plugin1';

const plugins = [Plugin1()];
const coreLibrary = CoreLibrary(plugins);
const [plugin1] = coreLibrary.getPlugins()

plugin1.someMethod();

Have you considered an API like that?

davidjerleke commented 2 years ago

Thanks for the response @xiel,

Initially I wanted to do something similar to your suggestion. However there's a problem I don't know how to solve with that approach. Consider this:

function EmblaCarousel<PluginsType>(
  rootNode: HTMLElement,
  options?: Partial<EmblaOptionsType>,
  plugins?: PluginsType
) {
  let activePlugins = plugins || []

 function getPlugins(): PluginsType {
   return activePlugins
 }

  return { getPlugins }
}

...However, as soon as the reInit function comes into play, this doesn't work because a reInit lets you pass new plugins which causes the initial provided PluginsType to become stale. I don't know if it's possible to have a dynamic generic type with TypeScript. The problem:

function EmblaCarousel<PluginsType>(
  rootNode: HTMLElement,
  options?: Partial<EmblaOptionsType>,
  plugins?: PluginsType
) {
  let activePlugins = plugins || []

  function reInit<PluginsType>(
    options?: Partial<EmblaOptionsType>, 
    plugins?: PluginsType
  ): void {
    activePlugins = plugins || activePlugins
  }

 function getPlugins(): PluginsType {
   return activePlugins
 }

  return { getPlugins }
}

Let me know if you know a way around this and thanks.

Cheers, David

davidjerleke commented 2 years ago

By the way, sorry @xiel for destroying the comment section in this PR with irrelevant stuff 🙈.

xiel commented 2 years ago

@davidjerleke All good 👍 You're right, this does not really work well with reinit.

I'm currently drafting a different approach, seems to work so far. I think I will open a PR today and we can discuss it further there.

xiel commented 2 years ago

@davidjerleke Here we go: https://github.com/davidjerleke/embla-carousel/pull/335 Let me know what you think. See you over there.