Fix JSON.stringify polyfill name #181

mohd-akram · 2023-09-18T23:38:55Z

Fixes #180

JLHwung

Thanks!

nicolo-ribaudo

Thank you!

mohd-akram · 2023-09-19T17:27:16Z

Fixed the tests.

nicolo-ribaudo · 2023-09-19T17:37:52Z

packages/babel-plugin-polyfill-corejs3/src/built-in-definitions.ts

@@ -385,7 +385,7 @@ export const StaticProperties: ObjectMap2<CoreJSPolyfillDescriptor> = {
      "es.object.create",
      "es.object.freeze",
    ]),
-    stringify: define("json/stringify", ["es.json.stringify"], "es.symbol"),
+    stringify: define("json/stringify", ["es.json.stringify"]),


It's possible that the reason there was es.symbol is that when polyfilling JSON.stringify we also have to polyfill symbols. In that case, this definition should be

Suggested change

stringify: define("json/stringify", ["es.json.stringify"]),

stringify: define("json/stringify", ["es.json.stringify", "es.symbol"]),

Let's wait for @zloirock to see this PR before merging :)

Thanks, fixed.

zloirock · 2023-09-19T17:43:06Z

packages/babel-plugin-polyfill-corejs3/src/built-in-definitions.ts

@@ -385,7 +385,7 @@ export const StaticProperties: ObjectMap2<CoreJSPolyfillDescriptor> = {
      "es.object.create",
      "es.object.freeze",
    ]),
-    stringify: define("json/stringify", ["es.json.stringify"], "es.symbol"),
+    stringify: define("json/stringify", ["es.json.stringify"]),


It's not a bug or something like that. In early versions of core-js@3, JSON.stringify logic was only in es.symbol module to fix JSON.stringify with symbols. After that, many other fixes of JSON.stringify logic, well-formed unicode, JSON.rawJSON support, etc. were added and the main part of JSON.stringify polyfill was moved to es.json.stringify.

For modern core-js versions, es.symbol here is not required, however, it could break compatibility with early core-js@3 versions.

zloirock · 2023-09-21T20:17:33Z

...plugin-polyfill-corejs3/test/fixtures/usage-pure/built-in-static-methods-proposals/output.js

@@ -2,7 +2,6 @@ var _Array$from = require("core-js-pure/features/array/from.js");
 var _Array$isArray = require("core-js-pure/features/array/is-array.js");
 var _Array$of = require("core-js-pure/features/array/of.js");
 var _Date$now = require("core-js-pure/features/date/now.js");
-var _JSON$stringify = require("core-js-pure/features/json/stringify.js");


It should not be removed with those options.

I think it's removed because it seems to default to version 3.0. I noticed this while testing the package.

This entry is available in 3.0.

IIRC it could be removed because of the order of modules in the array of dependencies - here the first is es.json.stringify that wasn't in 3.0 - however, it does not mean that core-js-pure/features/json/stringify is not required.

It checks getModulesByVersion (something like that, which references a JSON file with that metadata) in core-js-compat and es.json.stringify is not mentioned under 3.0. I think that's why.

For pure I believe it only checks the first entry in the array since a pure import will fetch its own dependencies.

Yes, because JSON.stringify polyfill was only in es.symbol and it was exported in core-js-pure/features/json/stringify. Which is what we're talking about above.

zloirock · 2023-09-21T20:31:55Z

I think that the way solving of this issue is wrong - it's better to allow passing entries for exclusion, like exclude: ['json/stringify'] or something like that.

mohd-akram · 2023-10-15T13:33:48Z

This PR is to fix the exclusion :D I think perhaps the library should default to using the latest version of core-js rather than 3.0.

mohd-akram · 2024-02-14T13:53:27Z

Any update on this?

JLHwung approved these changes Sep 19, 2023

View reviewed changes

JLHwung requested a review from zloirock September 19, 2023 02:02

nicolo-ribaudo approved these changes Sep 19, 2023

View reviewed changes

mohd-akram force-pushed the fix-json-stringify branch from 6de8ed4 to 61cd3f0 Compare September 19, 2023 17:25

nicolo-ribaudo reviewed Sep 19, 2023

View reviewed changes

zloirock reviewed Sep 19, 2023

View reviewed changes

mohd-akram force-pushed the fix-json-stringify branch from 61cd3f0 to db02e94 Compare September 20, 2023 12:58

zloirock reviewed Sep 21, 2023

View reviewed changes

Fix JSON.stringify polyfill name

88cb08a

mohd-akram force-pushed the fix-json-stringify branch from db02e94 to 88cb08a Compare August 30, 2024 20:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix JSON.stringify polyfill name #181

Fix JSON.stringify polyfill name #181

mohd-akram commented Sep 18, 2023

JLHwung left a comment

nicolo-ribaudo left a comment

mohd-akram commented Sep 19, 2023

nicolo-ribaudo Sep 19, 2023

mohd-akram Sep 20, 2023

zloirock Sep 19, 2023 •

edited

Loading

zloirock Sep 21, 2023

mohd-akram Sep 21, 2023

zloirock Sep 21, 2023 •

edited

Loading

mohd-akram Sep 21, 2023 •

edited

Loading

zloirock Sep 21, 2023 •

edited

Loading

zloirock commented Sep 21, 2023

mohd-akram commented Oct 15, 2023

mohd-akram commented Feb 14, 2024

	stringify: define("json/stringify", ["es.json.stringify"]),
	stringify: define("json/stringify", ["es.json.stringify", "es.symbol"]),

Fix JSON.stringify polyfill name #181

Are you sure you want to change the base?

Fix JSON.stringify polyfill name #181

Conversation

mohd-akram commented Sep 18, 2023

JLHwung left a comment

Choose a reason for hiding this comment

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

mohd-akram commented Sep 19, 2023

nicolo-ribaudo Sep 19, 2023

Choose a reason for hiding this comment

mohd-akram Sep 20, 2023

Choose a reason for hiding this comment

zloirock Sep 19, 2023 • edited Loading

Choose a reason for hiding this comment

zloirock Sep 21, 2023

Choose a reason for hiding this comment

mohd-akram Sep 21, 2023

Choose a reason for hiding this comment

zloirock Sep 21, 2023 • edited Loading

Choose a reason for hiding this comment

mohd-akram Sep 21, 2023 • edited Loading

Choose a reason for hiding this comment

zloirock Sep 21, 2023 • edited Loading

Choose a reason for hiding this comment

zloirock commented Sep 21, 2023

mohd-akram commented Oct 15, 2023

mohd-akram commented Feb 14, 2024

zloirock Sep 19, 2023 •

edited

Loading

zloirock Sep 21, 2023 •

edited

Loading

mohd-akram Sep 21, 2023 •

edited

Loading

zloirock Sep 21, 2023 •

edited

Loading