Skip to content

Conversation

@curtisman
Copy link
Contributor

No description provided.

@MSLaguana
Copy link
Contributor

Looks like you need to update a few baselines to reflect this change as well.

@curtisman
Copy link
Contributor Author

curtisman commented Jul 21, 2017

@MSLaguana The accessor function name needs to have the bracket. toStringTag already have similar pattern and defined propertyId for the function name with [] for all the symbols.. So I updated it to create the accessor with the appropriate name.

The debugger baseline will need to be updated :(


void JavascriptLibrary::AddSpeciesAccessorsToLibraryObject(DynamicObject* object, FunctionInfo * getterFunctionInfo)
{
if (scriptContext->GetConfig()->IsES6SpeciesEnabled())
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: species have been enabled since th2. We could get rid of this check.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We probably can do a clean up for all the enabled language feature later.

@akroshg
Copy link
Contributor

akroshg commented Jul 21, 2017

LGTM

1 similar comment
@aneeshdk
Copy link
Contributor

LGTM

@chakrabot chakrabot merged commit 278071f into chakra-core:release/1.6 Jul 21, 2017
chakrabot pushed a commit that referenced this pull request Jul 21, 2017
…houldn't have bracket around the name

Merge pull request #3412 from curtisman:fix3368
chakrabot pushed a commit that referenced this pull request Jul 21, 2017
…String() shouldn't have bracket around the name

Merge pull request #3412 from curtisman:fix3368
chakrabot pushed a commit that referenced this pull request Jul 21, 2017
…ol.species.toString() shouldn't have bracket around the name

Merge pull request #3412 from curtisman:fix3368
@curtisman curtisman deleted the fix3368 branch April 16, 2018 17:36
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.

6 participants