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

Symbol JSON.stringify returns immediately without calling replacer fn when target is undefined #345

Closed
christophercr opened this issue Oct 17, 2017 · 8 comments

Comments

@christophercr
Copy link

christophercr commented Oct 17, 2017

Hi, first of all thanks for this nice library, it is really handy for those struggling with cross-browser compatible apps or better said IE-proof apps :p

I recently stumbled upon an issue though. It took me a bit of time to troubleshoot but here it is:

I have an app in angularjs and it uses the latest angular-ui-router. Then, in this router library there is this line of code that tries to get an stringified object passing a replacer function to ultimately log something to the console.

The problem pops up in IE11, when the core-js Symbol polyfill is used; which means that also the JSON.stringify() method is wrapped to work correctly with Symbols.

At some point, the routing library will call the JSON.stringify method passing undefined and a replacer function, and then, the wrapped JSON.stringify() function simply returns without doing anything nor even calling the replacer function.

This behaviour seems totally different in the native implementation of compliant browsers like Chrome and Firefox where the replacer function is called even if the target is undefined.

Do you think this could be refactored so that the replacer function is called?

@christophercr
Copy link
Author

Hi @zloirock, do you think you can have a look on this issue? Is there any reason why the Symbol polyfill was implemented in that way so that the replacer function is not called?

@zloirock
Copy link
Owner

zloirock commented Dec 8, 2017

Thanks for the report, sorry for the delay.

@zloirock zloirock added the es6 label Dec 8, 2017
zloirock added a commit that referenced this issue Dec 8, 2017
zloirock added a commit that referenced this issue Dec 8, 2017
zloirock added a commit that referenced this issue Dec 9, 2017
zloirock added a commit that referenced this issue Dec 9, 2017
@zloirock
Copy link
Owner

zloirock commented Dec 9, 2017

Fixed in core-js@2.5.2.

zloirock added a commit that referenced this issue Dec 9, 2017
@zloirock zloirock added the bug label Dec 10, 2017
zloirock added a commit that referenced this issue Dec 10, 2017
zloirock added a commit that referenced this issue Dec 11, 2017
@romgar
Copy link

romgar commented Dec 11, 2017

When I use core-js@2.5.2, I get a:

undefined is not a constructor (evaluating '$replacer.call(this, key, value)')
replacer@webpack:///~/babel-runtime/~/core-js/library/modules/es6.symbol.js:219:0 <- test_bundler.js:91356:45
replacer@webpack:///~/babel-polyfill/~/core-js/modules/es6.symbol.js:219:0 <- test_bundler.js:538:45
stringify@[native code]
stringify@webpack:///~/babel-polyfill/~/core-js/modules/es6.symbol.js:223:0 <- test_bundler.js:542:29
stringify@webpack:///~/babel-runtime/~/core-js/library/modules/es6.symbol.js:223:0 <- test_bundler.js:91360:29
stringify@webpack:///~/babel-runtime/~/core-js/library/fn/json/stringify.js:4:0 <- test_bundler.js:147023:32

triggered by a call to JSON.stringify in one of my tests.

The tests are passing properly with core-js@2.5.1.

@zloirock
Copy link
Owner

@romeovs could you provide your input data? Something like that is possible with incorrect replacer argument.

zloirock added a commit that referenced this issue Dec 11, 2017
zloirock added a commit that referenced this issue Dec 11, 2017
zloirock added a commit that referenced this issue Dec 11, 2017
@romgar
Copy link

romgar commented Dec 12, 2017

@zloirock Thanks for your responsiveness!
I definitely had a weird input for the replacer (JSON.stringify(data, 2, 2)), caught in some legacy code.
Do you have any idea why did it previously work? Giving a proper value for the replacer fixed the issue.

@zloirock
Copy link
Owner

@romgar fixed in core-js@2.5.3.

@romgar
Copy link

romgar commented Dec 12, 2017

I confirm that it fixed it!
Thanks for the release and your work @zloirock !

zloirock added a commit that referenced this issue Dec 22, 2017
zloirock added a commit that referenced this issue Dec 24, 2017
zloirock added a commit that referenced this issue Dec 24, 2017
zloirock added a commit that referenced this issue Dec 29, 2017
zloirock added a commit that referenced this issue Jan 2, 2018
zloirock added a commit that referenced this issue Jan 3, 2018
zloirock added a commit that referenced this issue Jan 13, 2018
zloirock added a commit that referenced this issue Jan 24, 2018
zloirock added a commit that referenced this issue Feb 6, 2018
zloirock added a commit that referenced this issue Feb 16, 2018
zloirock added a commit that referenced this issue Feb 27, 2018
zloirock added a commit that referenced this issue Mar 15, 2018
zloirock added a commit that referenced this issue Mar 17, 2018
zloirock added a commit that referenced this issue Mar 27, 2018
zloirock added a commit that referenced this issue Mar 27, 2018
zloirock added a commit that referenced this issue Apr 3, 2018
zloirock added a commit that referenced this issue Apr 9, 2018
zloirock added a commit that referenced this issue Apr 21, 2018
zloirock added a commit that referenced this issue May 4, 2018
zloirock added a commit that referenced this issue May 5, 2018
zloirock added a commit that referenced this issue May 7, 2018
zloirock added a commit that referenced this issue May 14, 2018
zloirock added a commit that referenced this issue May 18, 2018
zloirock added a commit that referenced this issue May 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants