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

Add better error message for missing mocks #876

Closed JuhG closed 2 weeks ago

JuhG commented 1 month ago

Right now it's hard to figure out what's wrong here: Screenshot 2024-05-17 at 13 01 09 "undefined is not a function"

Almost everyone will run into this after upgrading to the new major version. Maybe we could add a better error message. Perhaps linking to this comment or directly to the mocks.

Looking at caniuse stats, it's pretty unrealistic that people will run into these errors unless running tests. What do you think?

davidjerleke commented 1 month ago

@JuhG thank you for your efforts and sorry about the delayed response.

While I think this is a good idea, I would like to see a different approach to solve this. Because the current solution will make it to the production bundle increasing the bundle size. I've been planning to create separate bundles for dev and prod where the dev bundle can include helpful error messages like this and many more. But I haven't had the time to change this yet.

JuhG commented 1 month ago

@davidjerleke that's true maybe you could try to add something like https://github.com/alexreardon/tiny-warning or just manually checking NODE_ENV:

if ('production' !== process.env.NODE_ENV) {
  warning(condition, 'My cool message that takes up a lot of kbs');
}

these messages disappear when bundlers do their thing, so realistically they won't increase the bundle size (so no need to create separate dev and prod bundles)

davidjerleke commented 1 month ago

@JuhG are you sure these if statements disappear automatically?

There are different bundlers out there like Webpack, Esbuild, RollupJS etc. Do all of them really strip these if statements automatically without the need of using plugins and configuring them? I would like to have proof that this is the case before I proceed with this.

JuhG commented 1 month ago

@davidjerleke at work we use webpack, I've tested that and it works. can't say anything on other bundlers, but it would make sense, since process.env.NODE_ENV can't change once a node process already started (I think) maybe we could make some tests with Vite (I think market leader at this point), but ultimately it's your decision, I don't want to force anything you :)

davidjerleke commented 2 weeks ago

@JuhG as mentioned I appreciate the effort but would need hard evidence that the most common build tools strip this automatically.