Skip to content

fix(auth): declare class before proxy construction - #2608

Closed
alx-andru wants to merge 1 commit into
angular:masterfrom
alx-andru:fix/2296-proxy-object-IE11
Closed

alx-andru wants to merge 1 commit into
angular:masterfrom
alx-andru:fix/2296-proxy-object-IE11

Conversation

@alx-andru

Copy link
Copy Markdown
Contributor

Checklist

Description

The new Proxy implementation is not natively compatible with IE11. The recommended polyfill proxy-polyfill has limitations and requires an object to have all properties assigned at creation time of the new proxy object.

The AngularFireAuth class inherits from the Auth interface (part of the firebase SDK) and only implements angular specific augmentation to the existing interface.

This PR re-declares the existing interface from the imported auth.Auth interface and assignes each property with null. This guarantees that properties are known to the proxy and can be invoked using the proxy-polyfill with IE11.

Further, es5 does not support Array.prototoype.includes. I wasn't sure how to workaround this, as the specific use-case is not complex, an indexOf was being used.

Code sample

N/A

@alx-andru

Copy link
Copy Markdown
Contributor Author

I would appreciate feedback on the proposed solution as I'd be happy to extend the PR to the other modules to provide a more complete solution as well as documentation on how to use AngularFire with IE11.

@jamesdaniels

Copy link
Copy Markdown
Contributor

Hmmm, I'm wondering if there's any way we could inject these null properties at built-time. That way it's a bit more automated & less of a maintenance burden.

@alx-andru

Copy link
Copy Markdown
Contributor Author

I'm not too familiar with the build process yet but I'd be happy to explore some options. Are there any approaches you'd be thinking of right away?

Would it be also an option to use the initial approach in order to allow developers to update the package and maintain IE11 support with the latest releases until a more efficient solution is identified?

@alx-andru

Copy link
Copy Markdown
Contributor Author

Just following up, I'd be able to go ahead and complete the interface re-declarations as a current approach. I wasn't able to find a feasible way to provide a more automated approach. If there are some ideas I'd be happy give them a try and implement accordingly.

@alx-andru

Copy link
Copy Markdown
Contributor Author

Based on the comments in #2296 it doesn't seem to be an approach to follow further as the implications for maintenance would exceed the benefits.

@alx-andru alx-andru closed this Nov 1, 2020
@jamesdaniels

Copy link
Copy Markdown
Contributor

@alx-andru feel free to toss your indexOf change in another PR, that doesn't seem like a maintenance burden :P I'm going to keep thinking on this, maybe I'll come up with something that isn't too hard... though with a new modular version of the JS SDK on the way I'll be rethinking many of the assumptions with the current API in the coming weeks.

@jamesdaniels jamesdaniels mentioned this pull request Nov 3, 2020
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.

3 participants