Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

set window target for minified build #685

Merged
merged 3 commits into from
Oct 24, 2019
Merged

set window target for minified build #685

merged 3 commits into from
Oct 24, 2019

Conversation

tinovyatkin
Copy link
Contributor

@tinovyatkin tinovyatkin commented Oct 23, 2019

This PR doing 3 important things:

  1. Taking in account that it's a browser-only library setting globalObject: 'window' for webpack

  2. Repointing main from package.json to non-minified UMD version and ships minified version as bounded to window function, not as UMD module.
    Motivation here: if some developer are using this library via import Choices from 'choices.js' then that developer expect are non-minified version of the library to become part of his bundle (as they may debug something or apply different minifier later, for example Closure Compiler and existing minifying will hurt). This expectation particularly set by common sense and by the most popular Javascript library of all times.
    As result, coupled with upgraded fuse.js it decreases minified bundle size.

  3. Committed affected minified builded asset (we should get rid of them from repository, seriously :-) )

@jshjohnson jshjohnson merged commit bef6743 into Choices-js:master Oct 24, 2019
@tinovyatkin tinovyatkin deleted the set-global-this branch October 24, 2019 17:25
@gregjacobs
Copy link

@jshjohnson Just hitting an issue with regards to this: React apps that include Choices which are server-side rendered throw an error because window is not defined on the server.

What was the issue with leaving the default global var for UMD?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants