Skip to content

ci: webpack@5 test #459

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Aug 30, 2019
Merged

ci: webpack@5 test #459

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 1 addition & 4 deletions azure-pipelines.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ jobs:
node-8-canary:
node_version: ^8.9.0
webpack_version: next
continue_on_error: true
steps:
- task: NodeTool@0
inputs:
Expand Down Expand Up @@ -108,7 +107,6 @@ jobs:
node-8-canary:
node_version: ^8.9.0
webpack_version: next
continue_on_error: true
steps:
- task: NodeTool@0
inputs:
Expand Down Expand Up @@ -163,7 +161,6 @@ jobs:
node-8-canary:
node_version: ^8.9.0
webpack_version: next
continue_on_error: true
steps:
- script: 'git config --global core.autocrlf input'
displayName: 'Config git core.autocrlf'
Expand Down Expand Up @@ -198,4 +195,4 @@ jobs:
displayName: 'Publish test results'
- script: curl -s https://codecov.io/bash | bash -s -- -t $(CODECOV_TOKEN)
condition: succeededOrFailed()
displayName: 'Submit coverage data to codecov'
displayName: 'Submit coverage data to codecov'
95 changes: 50 additions & 45 deletions lib/fs.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,63 +12,68 @@ const DevMiddlewareError = require('./DevMiddlewareError');
module.exports = {
toDisk(context) {
const compilers = context.compiler.compilers || [context.compiler];

for (const compiler of compilers) {
compiler.hooks.afterEmit.tap('WebpackDevMiddleware', (compilation) => {
const { assets } = compilation;
const { log } = context;
const { writeToDisk: filter } = context.options;
let { outputPath } = compiler;
compiler.hooks.emit.tap('WebpackDevMiddleware', (compilation) => {
compiler.hooks.assetEmitted.tapAsync(
'WebpackDevMiddleware',
(file, content, callback) => {
let targetFile = file;

if (outputPath === '/') {
outputPath = compiler.context;
}
const queryStringIdx = targetFile.indexOf('?');

for (const assetPath of Object.keys(assets)) {
let targetFile = assetPath;
if (queryStringIdx >= 0) {
targetFile = targetFile.substr(0, queryStringIdx);
}

const queryStringIdx = targetFile.indexOf('?');
let { outputPath } = compiler;

if (queryStringIdx >= 0) {
targetFile = targetFile.substr(0, queryStringIdx);
}
// TODO Why? Need remove in future major release
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this when we release the next version?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, i think we should drop this in next major, i keep this because i can't understand why it is here 😕 Any ideas?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it's a good idea which is keeping. I don't know why...
We'll drop this at other pr.

if (outputPath === '/') {
outputPath = compiler.context;
}

const targetPath = path.isAbsolute(targetFile)
? targetFile
: path.join(outputPath, targetFile);
const allowWrite =
filter && typeof filter === 'function' ? filter(targetPath) : true;
outputPath = compilation.getPath(outputPath, {});

if (allowWrite) {
const asset = assets[assetPath];
let content = asset.source();
const targetPath = path.join(outputPath, targetFile);

if (!Buffer.isBuffer(content)) {
// TODO need remove in next major release
if (Array.isArray(content)) {
content = content.join('\n');
}
const { writeToDisk: filter } = context.options;
const allowWrite =
filter && typeof filter === 'function'
? filter(targetPath)
: true;

content = Buffer.from(content, 'utf8');
if (!allowWrite) {
return callback();
}

mkdirp.sync(path.dirname(targetPath));

try {
fs.writeFileSync(targetPath, content, 'utf-8');

log.debug(
colors.cyan(
`Asset written to disk: ${path.relative(
process.cwd(),
targetPath
)}`
)
);
} catch (e) {
log.error(`Unable to write asset to disk:\n${e}`);
}
const { log } = context;
const dir = path.dirname(targetPath);

return mkdirp(dir, (mkdirpError) => {
if (mkdirpError) {
return callback(mkdirpError);
}

return fs.writeFile(targetPath, content, (writeFileError) => {
if (writeFileError) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we output Unable to write asset to disk: xxx as an error?

Copy link
Member Author

@alexander-akait alexander-akait Aug 30, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is unnecessary, we should fail compilation if something not written as expected, because it is potentially break application because file(s) can be used in backend

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, agree with you.

return callback(writeFileError);
}

log.debug(
colors.cyan(
`Asset written to disk: ${path.relative(
process.cwd(),
targetPath
)}`
)
);

return callback();
});
});
}
}
);
});
}
},
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/server-test/webpack.array.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ module.exports = [
{
test: /\.(svg|html)$/,
loader: 'file-loader',
query: { name: '[name].[ext]' },
options: { name: '[name].[ext]' },
},
],
},
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/server-test/webpack.client.server.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ module.exports = [
{
test: /\.(svg|html)$/,
loader: 'file-loader',
query: { name: '[name].[ext]' },
options: { name: '[name].[ext]' },
},
],
},
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/server-test/webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module.exports = {
{
test: /\.(svg|html)$/,
loader: 'file-loader',
query: { name: '[name].[ext]' },
options: { name: '[name].[ext]' },
},
],
},
Expand Down
2 changes: 1 addition & 1 deletion test/fixtures/server-test/webpack.querystring.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ module.exports = {
{
test: /\.(svg|html)$/,
loader: 'file-loader',
query: { name: '[name].[ext]' },
options: { name: '[name].[ext]' },
},
],
},
Expand Down
8 changes: 0 additions & 8 deletions test/server.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,16 +88,13 @@ describe('Server', () => {
request(app)
.get('/public/bundle.js')
.expect('Content-Type', 'application/javascript; charset=UTF-8')
// TODO(michael-ciniawsky) investigate the need for this test
.expect('Content-Length', '4631')
.expect(200, /console\.log\('Hey\.'\)/, done);
});

it('HEAD request to bundle file', (done) => {
request(app)
.head('/public/bundle.js')
.expect('Content-Type', 'application/javascript; charset=UTF-8')
.expect('Content-Length', '4631')
// eslint-disable-next-line no-undefined
.expect(200, undefined, done);
});
Expand All @@ -112,7 +109,6 @@ describe('Server', () => {
request(app)
.get('/public/svg.svg')
.expect('Content-Type', 'image/svg+xml; charset=UTF-8')
.expect('Content-Length', '4778')
.expect(200, done);
});

Expand Down Expand Up @@ -189,8 +185,6 @@ describe('Server', () => {
request(app)
.post('/public/bundle.js')
.expect('Content-Type', 'application/javascript; charset=UTF-8')
// TODO(michael-ciniawsky) investigate the need for this test
.expect('Content-Length', '4631')
.expect(200, /console\.log\('Hey\.'\)/, done);
});

Expand Down Expand Up @@ -248,8 +242,6 @@ describe('Server', () => {
it('GET request to bundle file', (done) => {
request(app)
.get('/bundle.js')
// TODO(michael-ciniawsky) investigate the need for this test
.expect('Content-Length', '4631')
.expect(200, /console\.log\('Hey\.'\)/, done);
});
});
Expand Down