-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Fix: ES5 client #2658
Merged
Merged
Fix: ES5 client #2658
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
577be40
test(client): improve es5 check
knagaitsev ee95fa9
chore(client): add comments to test and make test work
knagaitsev 19ebe17
refactor(client): make log and strip ansi transpiled bundle modules
knagaitsev fdedff8
test(client): add index bundle test
knagaitsev df200f0
chore(deps): update package-lock to fix lint
knagaitsev f33be55
test(client): fix bundle test to have right content type
knagaitsev File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
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
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,3 @@ | ||
'use strict'; | ||
|
||
module.exports = require('webpack/lib/logging/runtime'); |
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,3 @@ | ||
'use strict'; | ||
|
||
module.exports = require('strip-ansi'); |
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,39 @@ | ||
'use strict'; | ||
|
||
const path = require('path'); | ||
const merge = require('webpack-merge'); | ||
|
||
const base = { | ||
mode: 'production', | ||
output: { | ||
path: path.resolve(__dirname, '../../client/transpiled-modules'), | ||
libraryTarget: 'commonjs2', | ||
}, | ||
module: { | ||
rules: [ | ||
{ | ||
test: /\.js$/, | ||
use: [ | ||
{ | ||
loader: 'babel-loader', | ||
}, | ||
], | ||
}, | ||
], | ||
}, | ||
}; | ||
|
||
module.exports = [ | ||
merge(base, { | ||
entry: path.join(__dirname, 'log.js'), | ||
output: { | ||
filename: 'log.js', | ||
}, | ||
}), | ||
merge(base, { | ||
entry: path.join(__dirname, 'strip-ansi.js'), | ||
output: { | ||
filename: 'strip-ansi.js', | ||
}, | ||
}), | ||
]; | ||
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
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,79 @@ | ||
'use strict'; | ||
|
||
const fs = require('fs'); | ||
const path = require('path'); | ||
const acorn = require('acorn'); | ||
const request = require('supertest'); | ||
const testServer = require('../helpers/test-server'); | ||
const config = require('../fixtures/simple-config/webpack.config'); | ||
const port = require('../ports-map').bundle; | ||
const isWebpack5 = require('../helpers/isWebpack5'); | ||
|
||
describe('bundle', () => { | ||
// the ES5 check test for the bundle will not work on webpack@5, | ||
// because webpack@5 bundle output uses some ES6 syntax that can | ||
// only be avoided with babel-loader | ||
const runBundleTest = isWebpack5 ? describe.skip : describe; | ||
|
||
runBundleTest('index.bundle.js bundled output', () => { | ||
it('should parse with ES5', () => { | ||
const bundleStr = fs.readFileSync( | ||
path.resolve(__dirname, '../../client/default/index.bundle.js'), | ||
'utf8' | ||
); | ||
expect(() => { | ||
acorn.parse(bundleStr, { | ||
ecmaVersion: 5, | ||
}); | ||
}).not.toThrow(); | ||
}); | ||
}); | ||
|
||
runBundleTest('main.js bundled output', () => { | ||
let server; | ||
let req; | ||
|
||
beforeAll((done) => { | ||
server = testServer.start(config, { port }, done); | ||
req = request(server.app); | ||
}); | ||
|
||
afterAll(testServer.close); | ||
|
||
it('should get full user bundle and parse with ES5', async () => { | ||
const { text } = await req | ||
.get('/main.js') | ||
.expect('Content-Type', 'application/javascript; charset=utf-8') | ||
.expect(200); | ||
|
||
expect(() => { | ||
let evalStep = 0; | ||
acorn.parse(text, { | ||
ecmaVersion: 5, | ||
onToken: (token) => { | ||
// a webpack bundle is a series of evaluated JavaScript | ||
// strings like this: eval('...') | ||
// if we want the bundle to work using ES5, we need to | ||
// check that these strings are good with ES5 as well | ||
|
||
// this can be done by waiting for tokens during the main parse | ||
// then when we hit a string in an 'eval' function we also try | ||
// to parse that string with ES5 | ||
if (token.type.label === 'name' && token.value === 'eval') { | ||
evalStep += 1; | ||
} else if (token.type.label === '(' && evalStep === 1) { | ||
evalStep += 1; | ||
} else if (token.type.label === 'string' && evalStep === 2) { | ||
const program = token.value; | ||
acorn.parse(program, { | ||
ecmaVersion: 5, | ||
}); | ||
|
||
evalStep = 0; | ||
} | ||
}, | ||
}); | ||
}).not.toThrow(); | ||
}); | ||
}); | ||
}); |
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
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.
I am afraid, some modules can use new syntax (ES 6) and we can't catch it, for example in patch version
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.
@evilebottnawi If that happens when we are updating a dependency, it will be caught in CI here: https://github.com/webpack/webpack-dev-server/pull/2658/files#diff-24488ac612ba9666465a7a596040ac73R43
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.
But if we want full safety, we should do this: #2652. I don't like how that PR ended up because it has so many breaking changes in the API and seems a little risky
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.
Sound good to me. CI is enough for me.