Skip to content

Conversation

@kadamwhite
Copy link
Contributor

We want to be able to set output CSS files to be hashed by default to improve cacheability of generated assets, but to do that we also need to output a manifest file in all circumstances; otherwise, WordPress won't know how to locate the hashed CSS filename. This PR introduces logic to automatically insert a ManifestPlugin instance within the presets.production configuration factory, providing that no ManifestPlugin is already present.

@kadamwhite kadamwhite added the reviewer needed needs :eyes: label Jan 19, 2021
@goldenapples
Copy link
Contributor

One additional thing to point out in the documentation is that using the default production config for multiple builds in a directory, the last build to be run will overwrite the asset manifests from the other builds.

I'm not sure the best way to get around that, but this set of defaults doesn't seem safe as is without a clearer documentation of how to use manifest seeds to write asset paths for multiple webpack builds to the same manifest file.

@kadamwhite
Copy link
Contributor Author

using the default production config for multiple builds in a directory, the last build to be run will overwrite the asset manifests from the other builds

This is a good point, and a bigger issue than the actual "breaking" change I've described (which shouldn't impact any of our projects, per Sourcegraph). I wonder how much trouble it would be to use a consistent manifest seed per output directory...

@kadamwhite
Copy link
Contributor Author

Upon reflection, any project using the default manifest logic already has to account for this in a multi-config setup because the dev manifest will be subject to the same concerns. While automatically sharing a manifest for builds targeting the same directory is a nice value-add, I don't see it as a barrier to merging this, because I have not found any project which would be negatively impacted by the change.

In order to test this in a beta capacity, I am going to open a new issue for evaluating the shared seed option and will merge this for now.

@kadamwhite kadamwhite merged commit a94b2bb into main Jan 19, 2021
@kadamwhite kadamwhite deleted the generate-production-manifest branch January 19, 2021 23:21
kadamwhite added a commit that referenced this pull request Jan 19, 2021
@goldenapples notes in #153 that generating a production manifest
automatically could result in manifests being overwritten if two configs
share an output folder but do not specifically instruct their manifests
to use a consistent seed object. This commit changes the generation of
the production manifest to include a seed object which is unique per
output.path directory. In this way (validated by the changes to the
test-build npm script) multi-config webpack setups with multiple builds
targeting the same directory will share a manifest.
kadamwhite added a commit that referenced this pull request Feb 1, 2021
**New Features**

- Adapt the value of `output.path` when inferring `output.publicPath` in DevServer so that all assets are correctly served in multi-config situations. [#156](#156)
- Generate a `production-asset-manifest.json` for all production preset builds. [#153](#153) Builds in a multi-configuration setup which target the same output folder will share a manifest. [#154](#154)

**Upgrades & Changes**

- **Potentially Breaking**: Update `mini-css-extract-plugin` to v1.3.4. [Changelog](https://github.com/webpack-contrib/mini-css-extract-plugin/blob/master/CHANGELOG.md). [#148](#148)
- **Potentially Breaking**: Update `css-loader` to v5.0.1. [Changelog](https://github.com/webpack-contrib/css-loader/blob/master/CHANGELOG.md). [#136](#136)
- **Potentially Breaking**: Update `style-loader` to v2.0.0. [Changelog](https://github.com/webpack-contrib/style-loader/blob/master/CHANGELOG.md). [#127](#127)

All three of the above dependency upgrades include breaking API changes. None of these changes should impact vanilla usage of the [presets](https://humanmade.github.io/webpack-helpers/modules/presets.html) provided by these helpers, but we recommend reviewing all three libraries' changelogs to identify potential issues if you are [customizing presets or loader behavior](https://humanmade.github.io/webpack-helpers/modules/presets.html#customizing-presets) relating to stylesheets.

- **Potentially Breaking**: Update `webpack-manifest-plugin` to v3.0.0. [Changelog](https://github.com/shellscape/webpack-manifest-plugin/releases). [#143](#143)

The manifest plugin now exports its constructor as a named export, not a default, so any custom builds which pull in the `ManifestPlugin` constructor directly from the plugin dependency (as opposed to using the recommended `plugins.manifest()` factory function) will need to switch to using `plugins.constructors.ManifestPlugin`. This should not impact un-customized presets.

- **Potentially Breaking**: Update `postcss-flexbugs-fixes` to v5.0.2 and update `postcss-loader` to v4.1.0. [#152](#152)

The upgrade to `postcss-loader` requires nesting PostCSS configuration options within a `.postcssOptions` key on the object passed to the webpack loader. As with the style loading changes above, if you use the presets without customization this should not impact your project. Otherwise, ensure you have added this level of nesting to any code which customizes the `postcss-loader`'s configuration object. Consult the [`postcss-loader` changelog](https://github.com/webpack-contrib/postcss-loader/blob/master/CHANGELOG.md#-breaking-changes) for more information.

- Upgrade `webpack-fix-style-only-entries` plugin to v0.6.0. This may resolve the issue previously documented in [#93](#93) where files would be incorrectly deleted when processing an array of webpack configuration objects. [#129](#129)
- Include `postcss` as a direct dependency of this package, rather than a subdependency. [#151](#151)
- Update `webpack-bundle-analyzer` bundled plugin to v4.3.0. [#146](#146)

- Internal: Add the generation of a basic development and production bundle, including scss styles, to the CI job. [#149](#149)
- Internal: Upgrade local development dependencies to latest versions. [#150](#150)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

reviewer needed needs :eyes:

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants