Make optional jQuery require bundler-friendly (#4184)#4304
Make optional jQuery require bundler-friendly (#4184)#4304BigBalli wants to merge 1 commit intojashkenas:masterfrom
Conversation
The literal `require('jquery')` form (even inside try/catch) is
statically analyzed by Webpack, Rollup, esbuild and packd, so users
who do not actually want jQuery still see their bundles fail with
"Cannot find module 'jquery'".
Indirect the optional require through a variable that bundlers do
not follow:
var nodeRequire = typeof require === 'function' && require;
try { if (nodeRequire) $ = nodeRequire('jquery'); } catch (e) {}
Behavior is unchanged: jQuery remains optional, the try/catch still
swallows the runtime ENOENT in Node, and `Backbone.$` is still set
when jQuery is installed.
Also declare `jquery` as an optional peerDependency in package.json
so npm/yarn/pnpm document the relationship cleanly without forcing
the install.
|
Thank you for the awesome work in this pull request and the previous four! I made a superficial glance over all five PRs and they look very good to me. I will be reviewing them in more detail (and almost certainly also merging them) over the next two weeks or so. Do you expect any merge conflicts between them, and if so, which order would you recommend? |
jgonggrijp
left a comment
jgonggrijp
left a comment
There was a problem hiding this comment.
I'm happy with the modification in the UMD wrapper. 👍
Regarding the modification in the package.json, I have a question. I realize I suggested adding a peerDependecy, but now I wonder how much this might annoy users who are stuck with older versions of npm. I found these two threads about the introduction of peerDependenciesMeta:
peerDependenciesMeta appears to have been introduced in npm v6 or v7. As of v7, npm apparently went "back" to installing peerDependencies by default. Which implies that some older versions of npm, which did not recognize peerDependenciesMeta yet, would also install peerDependencies by default. Do you know which versions of npm are affected by this and how problematic the resulting behavior could be?
I can look into this as well, but I thought I'd ask in case you already know the answer.
2 participants
Fixes #4184.
Problem
The CommonJS branch of the UMD wrapper does:
The
try/catchis correct at runtime in Node.js, but Webpack, Rollup,esbuild, packd and other bundlers statically analyze the literal
require('jquery')call before the try/catch ever runs. Users whodon't actually want jQuery still see their bundles fail with
Cannot find module 'jquery'.Fix
Indirect the optional require through a variable so the bundler's
static analysis does not follow it:
This is the same trick used by
wsand other libraries with optionalnative deps. Runtime behavior is unchanged: the require still happens
in Node, the try/catch still swallows ENOENT, and
Backbone.$isstill populated when jQuery is installed.
Also declare
jqueryas an optional peer dependency inpackage.jsonso npm/yarn/pnpm document the relationship cleanly without forcing the
install:
Test
npm run lintpasses.require('./backbone.js')in a fresh Node process withno jQuery installed: loads cleanly,
Backbone.VERSION === '1.6.1',Backbone.$ === undefinedas expected.