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
Migrate package/source to ES module(s) #537
Merged
Changes from all commits
Commits
Show all changes
17 commits
Select commit
Hold shift + click to select a range
26a33eb
Turn source into ES module
carhartl f75545c
Introduce rollup
carhartl e3a09f3
Port uglify grunt task to rollup
carhartl 7382578
Add filesize rollup plugin
carhartl 6e90e54
Adapt eslint to account for module
carhartl 077d626
Adapt watch grunt task for module
carhartl a39aaa6
Adapt compare_size grunt task for module
carhartl 01e324c
Remove tests for amd + umd
carhartl 2be6618
Fix file references in tests
carhartl e1e1d90
Remove noConflict() testing remnants
carhartl a2c1136
Remove support for Bower
carhartl 0a7d1db
Set up package for module/nomodule distributions
carhartl d2a6f4c
Update qunit package, using latest
carhartl 0feef85
Fix typo
carhartl 60e9ed8
Add test for ES module
carhartl ac129d2
Bring back noConflict functionality for umd module
carhartl c96aea2
Adapt documentation for ES module usage
carhartl File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,4 @@ | ||
node_modules | ||
build | ||
dist | ||
.sizecache.json | ||
*.log* |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file was deleted.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,55 @@ | ||
import { terser } from "rollup-plugin-terser"; | ||
import filesize from "rollup-plugin-filesize"; | ||
import license from "rollup-plugin-license"; | ||
|
||
export default [ | ||
{ | ||
input: "src/js.cookie.mjs", | ||
output: [ | ||
// config for <script type="module"> | ||
{ | ||
dir: "dist", | ||
entryFileNames: "[name].mjs", | ||
format: "esm" | ||
}, | ||
// config for <script nomodule> | ||
{ | ||
dir: "dist", | ||
name: "Cookies", | ||
entryFileNames: "[name].js", | ||
format: "umd", | ||
noConflict: true | ||
} | ||
] | ||
}, | ||
{ | ||
input: "src/js.cookie.mjs", | ||
output: [ | ||
// config for <script type="module"> | ||
{ | ||
dir: "dist", | ||
entryFileNames: "[name].min.mjs", | ||
format: "esm" | ||
}, | ||
// config for <script nomodule> | ||
{ | ||
dir: "dist", | ||
name: "Cookies", | ||
entryFileNames: "[name].min.js", | ||
format: "umd", | ||
noConflict: true | ||
} | ||
], | ||
plugins: [ | ||
terser(), | ||
license({ | ||
banner: { | ||
content: | ||
"/*! <%= pkg.name %> v<%= pkg.version %> | <%= pkg.license %> */", | ||
commentStyle: "none" | ||
} | ||
}), | ||
filesize() | ||
] | ||
} | ||
]; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmmm enforcing consumers to use a JS file with a
.mjs
file extension at this level seems a little odd. Would all browsers know how to interpret a .mjs file extension?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't mean to force consumers, more like provide an example of how to include the module of that package directly, along with the fallback. In general, browsers do not care about the extension, though it's important a server serves
mjs
files with the proper mime type oftext/javascript
.Generally I'm assuming the more likely use case for ES modules is to be bundled up, so I might add that here...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Or do you mean it's too early to use
mjs
files in general? I was looking at https://v8.dev/features/modules and it seemed the way to go..There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AFAIK, mjs files only work with node out of the box. The only way browsers would parse a file with this extension is if, as you say, the
In other words, this won't work for consumers if they don't know to configure their server to serve the proper mime type. I would suggest we change the .mjs extension to .js like mentioned in #537 (comment). Then it wouldn't require a server change.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems the value for outputting a
.mjs
extension is to assist on documentation. Should we stick with one file for simplicity? Is there another reason for having both.js
and.mjs
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For now I'm siding with the node developers and wish to keep the extension:
I'm assuming it will be rather rare that the source will be consumed directly, rather than being bundled up with a tool. In the latter case the extension matters less, stressing the aforementioned arguments further.
Let's see how it goes. We can always change that back later on, but let's get some feedback first and start releasing a v3 beta once we're ready...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good, if it's a convention it doesn't hurt simply following it.