Skip to content

Fix IE11 broken by Crypto change. #2832

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

Merged
merged 3 commits into from
Mar 30, 2020
Merged

Fix IE11 broken by Crypto change. #2832

merged 3 commits into from
Mar 30, 2020

Conversation

wu-hui
Copy link
Contributor

@wu-hui wu-hui commented Mar 30, 2020

Fix for #2827

@@ -75,6 +75,8 @@ export class BrowserPlatform implements Platform {
}

const v = new Uint8Array(nBytes);
// Polyfill for IE11.
const crypto = window.crypto || (window as any).msCrypto;
Copy link
Member

Choose a reason for hiding this comment

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

You probably need to suppress eslint here to pass tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like you could do this once at the top of the file (after imports before the class).

@google-oss-bot
Copy link
Contributor

google-oss-bot commented Mar 30, 2020

Binary Size Report

Affected SDKs

SDKTypeBase (ef58845)Head (99bdfb1)Diff
@firebase/firestore/memorybrowser226115.00226387.00+272.00 (+0.12%)
module223295.00223567.00+272.00 (+0.12%)
esm2017171948.00172185.00+237.00 (+0.14%)
main399467.00400300.00+833.00 (+0.21%)
@firebase/firestorebrowser272688.00272960.00+272.00 (+0.10%)
module269467.00269739.00+272.00 (+0.10%)
esm2017269467.00269739.00+272.00 (+0.10%)
main490193.00491026.00+833.00 (+0.17%)
firebasefirebase-messaging.js39431.0039611.00+180.00 (+0.46%)
firebase.js847065.00847522.00+457.00 (+0.05%)
firebase-firestore.memory.js268418.00268695.00+277.00 (+0.10%)
firebase-firestore.js313659.00313936.00+277.00 (+0.09%)
@firebase/messagingmodule30549.0030886.00+337.00 (+1.10%)
esm201722806.0023151.00+345.00 (+1.51%)
main31006.0031343.00+337.00 (+1.09%)
Metric Unit: byte

Test Logs

Copy link
Contributor

@wilhuff wilhuff left a comment

Choose a reason for hiding this comment

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

LGTM

@@ -75,6 +75,8 @@ export class BrowserPlatform implements Platform {
}

const v = new Uint8Array(nBytes);
// Polyfill for IE11.
const crypto = window.crypto || (window as any).msCrypto;
Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like you could do this once at the top of the file (after imports before the class).

@wilhuff wilhuff assigned wu-hui and unassigned wilhuff Mar 30, 2020
@wu-hui wu-hui merged commit 353f5ca into master Mar 30, 2020
dconeybe added a commit that referenced this pull request Apr 6, 2020
@firebase firebase locked and limited conversation to collaborators Apr 30, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants