-
Notifications
You must be signed in to change notification settings - Fork 38
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Feedback #9
Comments
frehner
added a commit
that referenced
this issue
Aug 31, 2022
It seems like something updated, and now it supports the Correctly redirects to
Without any |
frehner
added a commit
that referenced
this issue
Sep 1, 2022
* Update sections and wording based on #9 * Add vite to the list of bundlers that understand dev/prod * Refer to bundlers in general * Slight text update on unpkg * Self review * Update text on minification to be more generic * Hedge a little more, because hedging is fun * Fix typo and wording for minifier text
# for free
to join this conversation on GitHub.
Already have an account?
# to comment
Some feedback from my initial read-through:
Output to esm, cjs, and umd formats
It might be helpful to use an example when explaining that
cjs
is sometimes worth catering to separately fromumd
. I'd suggest using the example of conditionalrequire()
based onprocess.env.NODE_ENV
, a common technique within the React ecosystem (because React itself uses this technique). These don't generally produce conditional dependencies when bundled fromumd
(conditions get ignored and dependencies included unconditionally).Don't minify
I'm not quite sure I agree with this section as-written. Application bundlers like Webpack do a significantly poorer job of minifying library code, and can't be configured to minify a given library in the way that would be most optimal. As an example, constant inlining is extremely valuable for performance and size when compiling (minifying) a library, but constant inlining within dependencies is only currently possible in Rollup with specific configuration.
Perhaps it would be worth breaking apart the minification advice. It's generally the case that application bundling+minifying setups do a reasonably effective job of minifying whitespace and local names, so libraries can skip those two forms of minification and produce a package containing relatively readable code. However, application bundling+minifying setups are generally much less effective at optimizing across module boundaries, and cannot make assumptions about code structure that are required to minify optimally.
I've generally tried to explain this as: if you're concerned about size and perf, you can compile your code before publishing using a tool like Terser, but configure it to perform optimizations rather than whitespace and name compression.
As an illustrative example: none of the current commonly-used minifiers will compress property names in the code from an npm module. Compressing/inlining/dissolving properties of internal objects within a library is a hugely valuable optimization for both size and performance. Packages that publish code without running optimizations like this are leaving these benefits on the floor.
Target modern browsers
It might be worth including a quick explanation of legacy browser support fallback in the TS example:
Define your exports
"script"
isn't used by anything, as far as I am aware of. I'm not sure why the Webpack docs mention it, webpack doesn't seem to ever use the property."module"
- it might be worth mentioning that this field's purpose is an optimization for systems that transparently handle CommonJSrequire()
of an ES Module, so thatrequire('x')
andimport 'x'
bundle/execute the same ES Module instead of resulting in two copies."default"
- there is generally no reason to specify both"require"
and"default"
, since there are no module systems that resolve Package Exports but lack support for both CommonJS and ESM."development"
+"production"
. Because these aren't supported by default in Rollup or Node, relying on them for nontrivial differentiation between dev/prod bundles is uncommon and a bit of a support/onboarding burden.Set the default module type for your JS files
It's a tiny detail, but I believe the existence of a
package.json
within a subdirectory currently will not override the default.js
file extension association if that file is reached by resolving from the"exports"
field of apackage.json
in a parent directory. I might be wrong on this though!Set the browser field
Not sure if it's worth mentioning, but unpkg.com currently ignores the
"browser"
field. You can set an"unpkg"
field to control which path unpkg will serve when a package is requested directly (unpkg.com/foo
). A lot of folks include both browser and unpkg, both pointing to a UMD bundle.The text was updated successfully, but these errors were encountered: