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

ESM #41

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

ESM #41

wants to merge 3 commits into from

Conversation

brettz9
Copy link
Contributor

@brettz9 brettz9 commented Aug 7, 2020

Builds on #40 (dropping bower).

  • Enhancement: Derive main easing files (including minified) from ESM source, with main ones assuming global jQuery
  • Enhancement: Add module, exports, browser, and type for ESM usage by bundlers or Node
  • Enhancement: Have CommonJS API require passing in jquery instance (passing in alone not sufficient)
  • Docs/Demo: Add Node CJS demo; give more on usage
  • Linting: More consistent styling
  • Refactoring: Switch to ES6 Modules in source
  • Refactoring: More ES6 in source
  • npm: Update jquery dep.; remove uglify-js (using Terser with Rollup)

A couple notes:

  1. If you are open to it, I can move the jquery.easing.js and jquery.easing.min.js to a new dist folder, as that should make things a bit cleaner, though it would increase breakage as the path would change.
  2. Since jQuery doesn't yet fully support ESM (e.g., see ESM distribution / exports in package.json jquery/jquery#4592 ), using import statements against jQuery in source would need our adding Rollup's commonjs plugin, and it would have the disadvantage of baking in a specific version of jQuery, adding to bundle size (unless we kept the import statement as an import statement, but then users would have to do their own bundling based on our bundle which isn't very convenient). So, currently, even one of the ESM source files relies on a jQuery global, though there is also another source file addJQueryEasing.js (in source at src/addJQueryEasing.mjs) which allows one to pass in one's own jQuery instance if that is desired. jQuery's current lack of proper ESM export is another reason I didn't bother with a native Node ESM demo, though I tentatively added such support in package.json (by the type and exports fields).
  3. I changed the API for CJS to require passing in a jQuery instance. This is because one really must have some DOM built to use in Node, e.g., with jsdom to get things working properly (I added a demo that executes without error, but as it is in Node, there is of course no visual effect). Note that CJS support had been broken until recently anyways, so I don't think this necessitates a breaking change (nor, as a result, should I think the addition of exports be so, even though it normally would be a breaking change since it prevents Node access to other files--but it wouldn't of course impact browser use even if installed by npm). module and browser can be breaking changes too, but I think they are targeting the files that should be expected.
  4. Although I didn't do so, we could add jquery as a peer dependency for Node usage, but Node really needs custom tooling like jsdom as well, so maybe better to avoid the hassle of having to keep peerDependencies up to date.
  5. While I added a few more linting rules for consistent styling, I'd be happy to apply a more rigorous config, e.g., standard, airbnb, etc., if you like.

… source, with main ones assuming global `jQuery`

- Enhancement: Add `module`, `exports`, `browser`, and `type` for ESM usage by bundlers or Node
- Enhancement: Have CommonJS API require passing in `jquery` instance (passing in alone not sufficient)
- Docs/Demo: Add Node CJS demo; give more on usage
- Linting: More consistent styling
- Refactoring: More on ES6 Modules source
- Refactoring: More ES6 in source
- npm: Update jquery dep.; remove uglify-js (using Terser with Rollup)
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.

1 participant