search

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

Code assumes the presence of `process` #164

Closed dermotduffy closed 2 years ago

dermotduffy commented 2 years ago

Thank you for this great plugin!

https://github.com/xiel/embla-carousel-wheel-gestures/blob/13aef98435d2e74de988a5bd14de4d97cbde69bc/embla-carousel-wheel-gestures/src/WheelGesturesPlugin.ts#L18

process may not be present in some environments, which causes:

Uncaught ReferenceError: process is not defined

Related: https://github.com/dermotduffy/frigate-hass-card/issues/531

davidjerleke commented 2 years ago

Hi @dermotduffy,

Thank you for creating this issue. Yes a null check could be added to prevent it from throwing. @xiel do you want to look into it or would you prefer a PR?

Best, David

xiel commented 2 years ago

@dermotduffy May I ask which version/bundle and packager you use?

Because this is a pretty common pattern and works with most packagers without problem and also helps with automatic code removal.

dermotduffy commented 2 years ago

Hi @xiel ,

Thank you for looking into this:

Rollup (config, package.json) is used to build a .js which is loaded in a browser.

    "rollup": "^2.70.1",
...
    "embla-carousel": "^6.2.0",
    "embla-carousel-wheel-gestures": "^2.1.1",

Let me know if you need anything else.

xiel commented 2 years ago

I think this is best solved by using the replace plugin by rollup. Please see my PR.

dermotduffy commented 2 years ago

Thank you for the PR on my repo! The downside of this approach is that it will fix it for me, but not the next person that runs into this problem with your repo.

Unless there is something fundamentally incompatible with how I'm using or bundling the plugin, perhaps adding the null check at source be a more future proof path forwards?

xiel commented 2 years ago

Most bundlers that target the browser handle this correctly out of the box (webpack, parcel, vite), so I don't expect many people running into this. Rollup is more barebone and needs to be configured correctly.

In case more people report it, we can reconsider.