Skip to content
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

Align webpack's outputPath with the whole app #1194

Merged
merged 2 commits into from
Apr 20, 2022
Merged

Conversation

ef4
Copy link
Contributor

@ef4 ef4 commented Apr 18, 2022

We hard-code webpack's output path to be the "assets" subdirectory of the built app (which appears as "./dist/assets" after you run a build).

This is needlessly confusing and limiting, because webpack's view of the world doesn't match what the app author actually writes. Mostly this doesn't matter when people are using all the defaults, but if they wanted to customize (for example) the location of the generated chunks they can't escape the assets subdir.

This PR fixes this problem by setting webpack's output.path to the root of the (output) ember app (meaning the "./dist" subdir in a typical build), and making corresponding adjustments to output.filename and output.chunkFileName so that we're still producing the same files by default, but they're now addressed relative to the whole app.

This is fix for #1157.

ef4 added 2 commits April 18, 2022 15:46
We hard-code webpack's output path to be the "assets" subdirectory of the built app (which appears as "./dist/assets" after you run a build).

This is needlessly confusing and limiting, because webpack's view of the world doesn't match what the app author actually writes. Mostly this doesn't matter when people are using all the defaults, but if they wanted to customize (for example) the location of the generated chunks they can't escape the assets subdir.

This PR fixes this problem by setting webpack's `output.path` to the root of the (output) ember app (meaning the "./dist" subdir in a typical build), and making corresponding adjustments to `output.filename` and `output.chunkFileName` so that we're still producing the same files by default, but they're now addressed relative to the whole app.
chunkFilename: `chunk.[chunkhash].js`,
publicPath: publicAssetURL + 'assets/',
path: join(this.outputPath),
filename: `assets/chunk.[chunkhash].js`,
Copy link
Collaborator

Choose a reason for hiding this comment

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

do we have existing tests to assert these files are created in this folder?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Tests likethese are directly inspecting the output files.

And the various tests/scenarios are ensuring that these files actually execute.

webpackConfig: Configuration;

// the base public URL for your assets in production. Use this when you want
// to serve all your assets from a different origin (like a CDN) than your
// actual index.html will be served on.
//
// This should be a URL ending in "/".
//
// For example:
Copy link
Collaborator

Choose a reason for hiding this comment

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

thanks for adding additional docs + examples!

@ef4 ef4 merged commit b59a578 into main Apr 20, 2022
@ef4 ef4 deleted the adjust-webpack-output-dir branch April 20, 2022 20:38
@rwjblue rwjblue added the bug Something isn't working label Apr 22, 2022
ef4 added a commit that referenced this pull request May 24, 2022
In #1194 we adjusted the webpack outputPath and made a compensating adjustment to the default JS chunk names so they would still end up in the same place. But we neglected to make a compensating change to the default chunk naming for production CSS chunks.

This caused the chunks to unintentionally move up one level in the directory hierarchy. This doesn't break anything, but it's inconsistent. This PR puts them back inside "assets" like before.
wondersloth pushed a commit to wondersloth/embroider that referenced this pull request Jun 1, 2022
In embroider-build#1194 we adjusted the webpack outputPath and made a compensating adjustment to the default JS chunk names so they would still end up in the same place. But we neglected to make a compensating change to the default chunk naming for production CSS chunks.

This caused the chunks to unintentionally move up one level in the directory hierarchy. This doesn't break anything, but it's inconsistent. This PR puts them back inside "assets" like before.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants