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

Unquoted object keys should support non-ASCII characters #1

Closed
aseemk opened this issue May 28, 2012 · 7 comments
Closed

Unquoted object keys should support non-ASCII characters #1

aseemk opened this issue May 28, 2012 · 7 comments
Assignees
Milestone

Comments

@EliSnow
Copy link

EliSnow commented May 25, 2014

Other references:
http://mathiasbynens.be/notes/javascript-properties
http://mothereff.in/js-properties (linked from above article)

@jordanbtucker
Copy link
Member

Are there any ideas on how to support Unicode and their escape sequences in identifiers? What are the options, and what are the pros and cons of those options?

It seems the biggest hurdle is that there is no standard way to determine what Unicode category a code point belongs to. For example, an identifier can start with $, _, or any character in the five Letter categories or in the Number, Letter category. That's 17196 different characters, all spread across the Unicode code point list.

Here are some ideas on perhaps how to solve this:

  • Require a dependency that provides support for testing Unicode categories.
  • Bake in support for it yourself. I've already done some prototyping and benchmarking.
  • Consume Unicode escape sequences and forego testing them against appropriate Unicode classes, willfully violating the ES5 spec.
  • Something very obvious I haven't thought of yet.

I think it's worth discussing the costs and benefits of implementing this support. What do you think?

@aseemk
Copy link
Member Author

aseemk commented Jul 29, 2014

Great work investigating this!

If you've already prototyped and profiled implementing this directly, I'd say that's a very appealing option. No dependency means this continues to be easily portable (e.g. in the browser).

👍

@rlidwka
Copy link
Contributor

rlidwka commented Aug 1, 2014

Full unicode testing could take as much space as the rest of the library. :)

But I think it's worth it. You could generate this using esprima tools and just hardcode it into the library, since it's unlikely to be changed, or just take this one.

PS: dependencies aren't an issue, because you can always use browserify.

@aseemk
Copy link
Member Author

aseemk commented Aug 3, 2014

Nice @rlidwka, thanks!

I agree re: browserify (we use and love it at FiftyThree), but I think there's still something to be said for having a simple drop-in file for developers who aren't knowledgeable about these tools. Maybe it's not worth worrying about that audience in this case though. We could also always provide a browserify-built distribution, either in the repo or via GitHub releases/downloads.

@ArcticLampyrid
Copy link

ArcticLampyrid commented Jun 10, 2017

Consume Unicode escape sequences and forego testing them against appropriate Unicode classes, willfully violating the ES5 spec.

Maybe this is enough.

@jordanbtucker jordanbtucker self-assigned this Sep 20, 2017
@jordanbtucker jordanbtucker added this to the v1.0.0 milestone Sep 20, 2017
@jordanbtucker
Copy link
Member

Fixed in 35269da.

jordanbtucker added a commit to jordanbtucker/json5 that referenced this issue Aug 11, 2022
This large commit enhances support for JavaScript modules.

This adds `lib/index.mjs`, which re-exports `lib/index.js` as a default
export and `parse` and `stringify` as named exports. The `module` field
in `package.json` now points to this file as the entry point for
`import` statements in versions of Node.js that support modules. With
this change, `import` statements in Node.js no longer need to use the
path `json5/dist/index.mjs`, and should import the module as `json5`
instead. `test/index.mjs` ensures that `lib/index.js` and
`lib/index.mjs` export the same instance. This follows the guidance in
approach json5#1 of [Writing dual packages while avoiding or minimizing
hazards][guidance].

`rollup.config.js` now uses `lib/index.mjs` as its entry point for
building browser bundles for JavaScript modules environments. It now
writes browser bundles to `dist/json5.js` and `dist/json5.esm.js`, as
well as minified versions. These are the new canonical paths for browser
bundles. The legacy `dist/index.*` paths still exist for backward
compatibility, but they will be removed in the next major version.

All files in `build` and `lib` now explicitly require `lib/index.js` to
ensure that `lib/index.mjs` is not imported. All files in `test` now
require the package root to simulate the default behavior of the version
of Node.js being tested.

[guidance]:
  https://nodejs.org/dist/latest-v16.x/docs/api/packages.html#writing-dual-packages-while-avoiding-or-minimizing-hazards
jordanbtucker added a commit to jordanbtucker/json5 that referenced this issue Aug 11, 2022
This large commit enhances support for JavaScript modules.

This adds `lib/index.mjs`, which re-exports `lib/index.js` as a default
export and `parse` and `stringify` as named exports. The `module` field
in `package.json` now points to this file as the entry point for
`import` statements in versions of Node.js that support modules. With
this change, `import` statements in Node.js no longer need to use the
path `json5/dist/index.mjs`, and should import the module as `json5`
instead. `test/index.mjs` ensures that `lib/index.js` and
`lib/index.mjs` export the same instance. This follows the guidance in
approach json5#1 of [Writing dual packages while avoiding or minimizing
hazards][guidance].

`rollup.config.js` now uses `lib/index.mjs` as its entry point for
building browser bundles for JavaScript modules environments. It now
writes browser bundles to `dist/json5.umd.js` and `dist/json5.esm.js`,
as well as minified versions. These are the new canonical paths for
browser bundles. The legacy `dist/index.*` paths still exist for
backward compatibility, but they will be removed in the next major
version.

All files in `build` and `lib` now explicitly require `lib/index.js` to
ensure that `lib/index.mjs` is not imported. All files in `test` now
require the package root to simulate the default behavior of the version
of Node.js being tested.

[guidance]:
  https://nodejs.org/dist/latest-v16.x/docs/api/packages.html#writing-dual-packages-while-avoiding-or-minimizing-hazards

squash
jordanbtucker added a commit to jordanbtucker/json5 that referenced this issue Aug 11, 2022
This large commit enhances support for JavaScript modules.

This adds `lib/index.mjs`, which re-exports `lib/index.js` as a default
export and `parse` and `stringify` as named exports. The `module` field
in `package.json` now points to this file as the entry point for
`import` statements in versions of Node.js that support modules. With
this change, `import` statements in Node.js no longer need to use the
path `json5/dist/index.mjs`, and should import the module as `json5`
instead. `test/index.mjs` ensures that `lib/index.js` and
`lib/index.mjs` export the same instance. This follows the guidance in
approach json5#1 of [Writing dual packages while avoiding or minimizing
hazards][guidance].

`rollup.config.js` now uses `lib/index.mjs` as its entry point for
building browser bundles for JavaScript modules environments. It now
writes browser bundles to `dist/json5.umd.js` and `dist/json5.esm.js`,
as well as minified versions. These are the new canonical paths for
browser bundles. The legacy `dist/index.*` paths still exist for
backward compatibility, but they will be removed in the next major
version.

All files in `build` and `lib` now explicitly require `lib/index.js` to
ensure that `lib/index.mjs` is not imported. All files in `test` now
require the package root to simulate the default behavior of the version
of Node.js being tested.

[guidance]:
  https://nodejs.org/dist/latest-v16.x/docs/api/packages.html#writing-dual-packages-while-avoiding-or-minimizing-hazards

squash
jordanbtucker added a commit to jordanbtucker/json5 that referenced this issue Aug 12, 2022
This large commit enhances support for JavaScript modules.

This adds `lib/index.mjs`, which re-exports `lib/index.js` as a default
export and `parse` and `stringify` as named exports. The `module` field
in `package.json` now points to this file as the entry point for
`import` statements in versions of Node.js that support modules. With
this change, `import` statements in Node.js no longer need to use the
path `json5/dist/index.mjs`, and should import the module as `json5`
instead. `test/index.mjs` ensures that `lib/index.js` and
`lib/index.mjs` export the same instance. This follows the guidance in
approach json5#1 of [Writing dual packages while avoiding or minimizing
hazards][guidance].

`rollup.config.js` now uses `lib/index.mjs` as its entry point for
building browser bundles for JavaScript modules environments. It now
writes browser bundles to `dist/json5.umd.js` and `dist/json5.esm.js`,
as well as minified versions. These are the new canonical paths for
browser bundles. The legacy `dist/index.*` paths still exist for
backward compatibility, but they will be removed in the next major
version.

All files in `build` and `lib` now explicitly require `lib/index.js` to
ensure that `lib/index.mjs` is not imported. All files in `test` now
require the package root to simulate the default behavior of the version
of Node.js being tested.

[guidance]:
  https://nodejs.org/dist/latest-v16.x/docs/api/packages.html#writing-dual-packages-while-avoiding-or-minimizing-hazards

squash
jordanbtucker added a commit to jordanbtucker/json5 that referenced this issue Aug 12, 2022
This large commit enhances support for JavaScript modules.

This adds `lib/index.mjs`, which re-exports `lib/index.js` as a default
export and `parse` and `stringify` as named exports. The `module` field
in `package.json` now points to this file as the entry point for
`import` statements in versions of Node.js that support modules. With
this change, `import` statements in Node.js no longer need to use the
path `json5/dist/index.mjs`, and should import the module as `json5`
instead. `test/index.mjs` ensures that `lib/index.js` and
`lib/index.mjs` export the same instance. This follows the guidance in
approach json5#1 of [Writing dual packages while avoiding or minimizing
hazards][guidance].

`rollup.config.js` now uses `lib/index.mjs` as its entry point for
building browser bundles for JavaScript modules environments. It now
writes browser bundles to `dist/json5.umd.js` and `dist/json5.esm.js`,
as well as minified versions. These are the new canonical paths for
browser bundles. The legacy `dist/index.*` paths still exist for
backward compatibility, but they will be removed in the next major
version.

All files in `build` and `lib` now explicitly require `lib/index.js` to
ensure that `lib/index.mjs` is not imported. All files in `test` now
require the package root to simulate the default behavior of the version
of Node.js being tested.

[guidance]:
  https://nodejs.org/dist/latest-v16.x/docs/api/packages.html#writing-dual-packages-while-avoiding-or-minimizing-hazards

squash
aseemk added a commit that referenced this issue Oct 1, 2022
* Explain JSON5 better. Proactively emphasize that this is for config file type use cases, not machine-to-machine communication (the #1 misunderstanding or objection to JSON5).

* Market/sell it better. The stats and company/project names are exciting! =)

* Link to blog post. This doesn't need to be front-and-center though; just added it to my name in the credits.

* Emphasize ES5 compatibility. Just put a stake in the ground here and preempt feature requests we'll never accept.
aseemk added a commit that referenced this issue Oct 1, 2022
* Explain JSON5 better. Proactively emphasize that this is for config file type use cases, not machine-to-machine communication (the #1 misunderstanding or objection to JSON5).

* Market/sell it better. The stats and company/project names are exciting! =)

* Link to blog post. This doesn't need to be front-and-center though; just added it to my name in the credits.

* Emphasize ES5 compatibility. Just put a stake in the ground here and preempt feature requests we'll never accept.
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

5 participants