-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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: add optional polyfill for crypto.randomUUID #8109
Conversation
Thank you very much for the quick solution! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks good to me.
It's also possible for @ember-data/store
to set its own macro config based off it's own decisions, like checking for the presence of a package or some other ember-data-specific configuration. But exposing the macros config directly to users is also supported and avoids the need for a lot of extra code.
Asset Size Report for 9993d55 Modern Builds ☑️ EmberData has not changed in sizeIf any packages had changed sizes they would be listed here. Changeset
Full Asset Analysis (Modern)
Modern Builds (No Rollup) ☑️ EmberData has not changed in sizeIf any packages had changed sizes they would be listed here. Changeset
Full Asset Analysis (Modern)
|
Performance Report for 9993d55 Scenario - materialization: ☑️ Performance is stable
Scenario - unload: ☑️ Performance is stable
Scenario - destroy: ☑️ Performance is stable
Scenario - add-children: ☑️ Performance is stable
Scenario - unused-relationships: ☑️ Performance is stable
|
Co-authored-by: Chris Thoburn <[email protected]>
Shouldnt this be the default? IMHO removing the support for insecure contexts without a changed configuration is kinda a breaking change 🤔 At least we should have a better error message, helping how to avoid this issue. |
@luxferresum opened #8301 for this ^ |
I spent a couple hours this morning trying to enable the polyfill as described above, until I realized that I needed to install
It may seem obvious to some, but not to me. So I am adding this here for anyone else encountering the same issue :) |
@gzurbach yeah, you only need to do that if we don't forward the config (which we should do). Haven't had time to work on that but if you do it would be hugely appreciated, shouldn't be too complicated we already forward the configs for near everything else. |
resolves #8097
resolves #8106
To use the polyfill: