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

Fingerprint scripts and styles in production #621

Merged
merged 3 commits into from
Nov 26, 2020

Conversation

simonihmig
Copy link
Collaborator

Fixes #289

Tested it locally in an app (ember s --prod).
Do we have any test coverage here for the stage 3 part, especially the production variant? AFAICT we only have one test-package that runs its tests in a production build in CI.

@simonihmig
Copy link
Collaborator Author

There is one issue here, that I could observe locally. However I believe this was just uncovered by the renaming of files, but the root cause is elsewhere. Created this issue: #622

@simonihmig
Copy link
Collaborator Author

AFAICT we only have one test-package that runs its tests in a production build in CI.

And it seems exactly this is failing here 🙈
Will need to investigate...

@simonihmig
Copy link
Collaborator Author

And it seems exactly this is failing here

Seems we have to explicitly exclude testem.js from fingerprinting, otherwise testing in a production environment does not work. Note that ember s --prod works (when opening /tests in the browser), it seems just the testem integration is broken when fingerprinting everything.

A quick Google search revealed a similar fix for ember-cli in this old PR: https://github.com/ember-cli/ember-cli/pull/1230/files#diff-8098951b282ee3680dd2afa8d34b95c7e542d90d65961bb60cbf897779c54eadR411

We already have data-embroider-ignore for this purpose.
@ef4 ef4 merged commit 2461223 into embroider-build:master Nov 26, 2020
@ef4
Copy link
Contributor

ef4 commented Nov 26, 2020

Thanks. I used the existing data-embroider-ignore feature to avoid needing to special-case testem. I think it's reasonable to require people to annotate scripts that they don't want to go through the build.

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.

Is there a way to fingerprint vendor.js?
3 participants