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

fix(addon-order): run esbuild before broccoli-asset-rev #61

Merged
merged 5 commits into from
Dec 1, 2021

Conversation

NullVoxPopuli
Copy link
Owner

@NullVoxPopuli NullVoxPopuli commented Dec 1, 2021

NOTE: PR diff is big because I added an addon for testing fingerprinting (from broccoli-asset-rev) in vendor.js.

This is a weird issue, because there is clearly a problem somewhere in the ecosystem, but images not being fingerprinted with ember-cli-esbuild-minifer currently has no reproduction local to this repo.

I tried to re-create the scenario that causes images to not be fingerprinted when ember-cli-esbuild-minifier is present, but was unable to do so.

There are additional assertions added that at least ensure images are fingerprinted -- checking both app and vendor js.

let app = await appJS();
let vendor = await vendorJS();

// fingerprint occurred
Copy link
Owner Author

Choose a reason for hiding this comment

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

these are the new assertions -- and I also verified this manually via ember s and opening the inspector

@NullVoxPopuli NullVoxPopuli merged commit e412882 into main Dec 1, 2021
"after": "broccoli-asset-rev"
"before": [
"ember-cli-sri",
"broccoli-asset-rev"
Copy link
Owner Author

Choose a reason for hiding this comment

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

this change is untested, but I believe this is the correct behavior

@github-actions
Copy link

github-actions bot commented Dec 1, 2021

🎉 This PR is included in version 5.1.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

1 participant