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(zone.js): read Symbol.species safely #45369

Closed
wants to merge 1 commit into from

Conversation

arturovt
Copy link
Contributor

@arturovt arturovt commented Mar 16, 2022

We must read Symbol.species safely because this may be anything. For instance, this
may be an object without a prototype (created through Object.create(null)); thus
this.constructor will be undefined. One of the use cases is SystemJS creating
prototype-less objects (modules) via Object.create(null). The SystemJS creates an empty
object and copies promise properties into that object (within the getOrCreateLoad
function). The zone.js then checks if the resolved value has the then method and invokes
it with the value context. Otherwise, this will throw an error: TypeError: Cannot read properties of undefined (reading 'Symbol(Symbol.species)').

PR Checklist

PR Type

  • Bugfix

Does this PR introduce a breaking change?

  • Yes
  • No

@pullapprove pullapprove bot requested a review from JiaLiPassion March 16, 2022 19:47
@arturovt arturovt force-pushed the fix/zonejs-symbol-species branch 2 times, most recently from d34c87c to 3832751 Compare March 16, 2022 23:19
@JiaLiPassion
Copy link
Contributor

@arturovt , thank you for the PR, could you add a test case around here

it('should allow subclassing without Symbol.species', () => {
?

Copy link
Contributor

@JiaLiPassion JiaLiPassion left a comment

Choose a reason for hiding this comment

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

Need to add test case

@arturovt arturovt force-pushed the fix/zonejs-symbol-species branch 6 times, most recently from 09a6baa to 2ae2267 Compare March 17, 2022 12:36
We must read `Symbol.species` safely because `this` may be anything. For instance, `this`
may be an object without a prototype (created through `Object.create(null)`); thus
`this.constructor` will be undefined. One of the use cases is SystemJS creating
prototype-less objects (modules) via `Object.create(null)`. The SystemJS creates an empty
object and copies promise properties into that object (within the `getOrCreateLoad`
function). The zone.js then checks if the resolved value has the `then` method and invokes
it with the `value` context. Otherwise, this will throw an error: `TypeError: Cannot read
properties of undefined (reading 'Symbol(Symbol.species)')`.
Copy link
Contributor

@JiaLiPassion JiaLiPassion left a comment

Choose a reason for hiding this comment

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

LGTM!

@JiaLiPassion JiaLiPassion added area: zones target: patch This PR is targeted for the next patch release labels Mar 20, 2022
@ngbot ngbot bot modified the milestone: Backlog Mar 20, 2022
@dylhunn
Copy link
Contributor

dylhunn commented Mar 24, 2022

@arturovt @JiaLiPassion Is this ready to be merged?

@JiaLiPassion JiaLiPassion added the action: merge The PR is ready for merge by the caretaker label Mar 25, 2022
@JiaLiPassion
Copy link
Contributor

@dylhunn , yes, this one is ready to merge, thank you.

@dylhunn
Copy link
Contributor

dylhunn commented Mar 25, 2022

This PR was merged into the repository by commit e2eaac3.

@dylhunn dylhunn closed this in e2eaac3 Mar 25, 2022
dylhunn pushed a commit that referenced this pull request Mar 25, 2022
We must read `Symbol.species` safely because `this` may be anything. For instance, `this`
may be an object without a prototype (created through `Object.create(null)`); thus
`this.constructor` will be undefined. One of the use cases is SystemJS creating
prototype-less objects (modules) via `Object.create(null)`. The SystemJS creates an empty
object and copies promise properties into that object (within the `getOrCreateLoad`
function). The zone.js then checks if the resolved value has the `then` method and invokes
it with the `value` context. Otherwise, this will throw an error: `TypeError: Cannot read
properties of undefined (reading 'Symbol(Symbol.species)')`.

PR Close #45369
@arturovt arturovt deleted the fix/zonejs-symbol-species branch March 25, 2022 01:58
PiyushAgrawal1243 pushed a commit to PiyushAgrawal1243/angular that referenced this pull request Mar 30, 2022
We must read `Symbol.species` safely because `this` may be anything. For instance, `this`
may be an object without a prototype (created through `Object.create(null)`); thus
`this.constructor` will be undefined. One of the use cases is SystemJS creating
prototype-less objects (modules) via `Object.create(null)`. The SystemJS creates an empty
object and copies promise properties into that object (within the `getOrCreateLoad`
function). The zone.js then checks if the resolved value has the `then` method and invokes
it with the `value` context. Otherwise, this will throw an error: `TypeError: Cannot read
properties of undefined (reading 'Symbol(Symbol.species)')`.

PR Close angular#45369
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Apr 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker area: zones target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants