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

Add support for new webpack asset manifest format #42

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

aeaston
Copy link

@aeaston aeaston commented Mar 2, 2021

Fixes #39

webpack-asset-manifest introduced a new format in v4. In order to support prefetch and preload, assets are now nested under an assets key in the entrypoints object. This commit updates the manifest parsing logic to support this new version in addition to the previous.

More info on the manifest changes can be found here

NOTE: after making these changes, I noticed Szeliga has a PR that also solves this problem. I think the approach here is a lot simpler than adding a configuration option, although I suppose that might be more extensible in the future. I don't expect manifest formats to change often though, so I still prefer this solution personally. But merging either would solve my problem and make me happy =)

webpack-asset-manifest introduced a new format in v4. In order to
support prefetch and preload, assets are not nested under an assets key
in the entrypoints object. This commit updates the manifest parsing
logic to support this new version in addition to the previous.

More info on the manifest changes can be found at
https://www.npmjs.com/package/webpack-assets-manifest#new-in-version-4
Copy link

@drewdrewthis drewdrewthis left a comment

Choose a reason for hiding this comment

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

LGTM -- I hope we can get this merged.

let(:name) { 'application.js' }
let(:type) { nil }

Choose a reason for hiding this comment

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

Since this seems like a default, you can probably leave it up in the describe scope and override it as needed (like on line 37)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Gem doesn't work with webpack-assets-manifest
2 participants