-
Notifications
You must be signed in to change notification settings - Fork 3.2k
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
chore: update to webpack v5 #27438
chore: update to webpack v5 #27438
Changes from all commits
a7c7f8e
d62dac9
b4cbe0a
de6a53d
3a152ac
23ff811
82a9b73
943912e
b723bc4
09643d9
bcc5f40
3520bc4
ed6e780
e49fc37
b39dae1
3e9fa7c
d445f61
d88f42d
6d8400e
23a42d0
89e6089
3406525
e62e7dd
0445987
e05801a
2e39e37
8ad7959
d8ccfc8
50dbba2
14118a2
cb5fb4e
3f0a2d5
ef87f97
77d81c2
a50ebdc
dd78d50
721d6a5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,26 @@ | ||
/** | ||
* as of Webpack 5, dependencies that are polyfilled through the Provide plugin must be defined inside the CLI | ||
* in order to guarantee there is a version of the dependency accessible by the cypress CLI, either in the cypress directory | ||
* or the root of their project. Currently, these two dependencies are 'buffer' and 'process' | ||
*/ | ||
describe('dependencies', () => { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I have some questions about this. If webpack 4 polyfilled all core Node.js libs out of the box, does that mean you could (and can)
I haven't tried this yet, but worth considering. I know we don't do this in our code base, but I'm sure some code bases rely on those polyfills (especially I wonder if we need to polyfill ALL the core modules to maintain backwards compatibility. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think it depends. |
||
it('process dependency exists in package.json and is available', () => { | ||
const { dependencies } = require('../../../package.json') | ||
|
||
expect(dependencies.process).to.be.ok | ||
|
||
const process = require('process') | ||
|
||
expect(typeof process).to.equal('object') | ||
}) | ||
|
||
it('buffer dependency exists in package.json and is available', () => { | ||
const { dependencies } = require('../../../package.json') | ||
|
||
expect(dependencies.buffer).to.be.ok | ||
|
||
const buffer = require('buffer') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we assert that require('node:buffer') !== require('buffer') That would verify the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that might be a good idea, considering we really want to just make sure the |
||
|
||
expect(typeof buffer).to.equal('object') | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,5 @@ | ||
const path = require('path') | ||
const webpack = require('webpack') | ||
const webpackPreprocessor = require('@cypress/webpack-preprocessor') | ||
|
||
const hasTsLoader = (rules) => { | ||
|
@@ -131,23 +132,51 @@ const getDefaultWebpackOptions = () => { | |
loader: require.resolve('coffee-loader'), | ||
}], | ||
}, | ||
plugins: [ | ||
new webpack.ProvidePlugin({ | ||
Buffer: ['buffer', 'Buffer'], | ||
process: 'process/browser', | ||
}), | ||
], | ||
resolve: { | ||
extensions: ['.js', '.json', '.jsx', '.mjs', '.coffee'], | ||
alias: { | ||
'child_process': require.resolve('./empty'), | ||
'cluster': require.resolve('./empty'), | ||
'console': require.resolve('./empty'), | ||
'dgram': require.resolve('./empty'), | ||
'dns': require.resolve('./empty'), | ||
'fs': require.resolve('./empty'), | ||
'http2': require.resolve('./empty'), | ||
'inspector': require.resolve('./empty'), | ||
'module': require.resolve('./empty'), | ||
'net': require.resolve('./empty'), | ||
'perf_hooks': require.resolve('./empty'), | ||
'readline': require.resolve('./empty'), | ||
'repl': require.resolve('./empty'), | ||
'tls': require.resolve('./empty'), | ||
fallback: { | ||
lmiller1990 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
assert: require.resolve('assert/'), | ||
buffer: require.resolve('buffer/'), | ||
child_process: false, | ||
cluster: false, | ||
console: false, | ||
constants: require.resolve('constants-browserify'), | ||
crypto: require.resolve('crypto-browserify'), | ||
dgram: false, | ||
dns: false, | ||
domain: require.resolve('domain-browser'), | ||
events: require.resolve('events/'), | ||
fs: false, | ||
http: require.resolve('stream-http'), | ||
https: require.resolve('https-browserify'), | ||
http2: false, | ||
inspector: false, | ||
module: false, | ||
net: false, | ||
os: require.resolve('os-browserify/browser'), | ||
path: require.resolve('path-browserify'), | ||
perf_hooks: false, | ||
punycode: require.resolve('punycode/'), | ||
process: require.resolve('process/browser'), | ||
querystring: require.resolve('querystring-es3'), | ||
readline: false, | ||
repl: false, | ||
stream: require.resolve('stream-browserify'), | ||
string_decoder: require.resolve('string_decoder/'), | ||
sys: require.resolve('util/'), | ||
timers: require.resolve('timers-browserify'), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oh, maybe we do polyfill them all. My previous comment may be irrelevant. Big breaking change here - who knows if these implementations match whatever webpack 4 was doing. Tracking down the right polyfill here must have been complex. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
This for sure breaks the
this gist for sure helped to figure out what was going on |
||
tls: false, | ||
tty: require.resolve('tty-browserify'), | ||
url: require.resolve('url/'), | ||
util: require.resolve('util/'), | ||
vm: require.resolve('vm-browserify'), | ||
zlib: require.resolve('browserify-zlib'), | ||
}, | ||
plugins: [], | ||
}, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -324,10 +324,21 @@ const preprocessor: WebpackPreprocessor = (options: PreprocessorOptions = {}): F | |
// this event is triggered when watching and a file is saved | ||
const onCompile = () => { | ||
debug('compile', filePath) | ||
const nextBundle = utils.createDeferred<string>() | ||
/** | ||
* Webpack 5 fix: | ||
* If the bundle is the initial bundle, do not create the deferred promise | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice job figuring this out 💯 |
||
* as we already have one from above. Creating additional deferments on top of | ||
* the first bundle causes reference issues with the first bundle returned, meaning | ||
* the promise that is resolved/rejected is different from the one that is returned, which | ||
* makes the preprocessor permanently hang | ||
*/ | ||
if (!bundles[filePath].initial) { | ||
const nextBundle = utils.createDeferred<string>() | ||
|
||
bundles[filePath].promise = nextBundle.promise | ||
bundles[filePath].deferreds.push(nextBundle) | ||
} | ||
|
||
bundles[filePath].promise = nextBundle.promise | ||
bundles[filePath].deferreds.push(nextBundle) | ||
bundles[filePath].promise.finally(() => { | ||
debug('- compile finished for %s, initial? %s', filePath, bundles[filePath].initial) | ||
// when the bundling is finished, emit 'rerun' to let Cypress | ||
|
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.
Would they also need to install webpack@4 to keep things working the same?
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.
not in this case because
v2
ofwebpack-batteries-included-preprocesor
ships with 4. its not backwards compatible unlike the regularwebpack-preprocessor