Subsume json #7985

jridgewell · 2018-05-19T01:01:50Z

https://github.com/tc39/proposal-json-superset is at stage 3.

This allows U+2028 LINE SEPARATOR and U+2029 PARAGRAPH SEPARATOR to appear unescaped inside strings and directives.

jridgewell · 2018-05-19T01:02:43Z

.../babel-plugin-proposal-json-strings/test/fixtures/json-strings/string-line-separator/exec.js

+expect(" ".length).toBe(1);
+//      ^ That's a U+2028 LINE SEPARATOR UTF-16 char
+
+expect("\ ".length).toBe(0);


Maybe someone more familiar with the spec can explain why \ followed by U+2028 gets parsed as an empty string.

Huh, that seems like a bug. The proposal doesn't appear to change the behavior for escaped unicode characters, just unescaped ones, and JSON.parse("\\\u2028") throws, so this probably should too.

This isn't a parse issue, I'm reasonably certain this is according to the spec.

✓ jridgewell@Dandy ~/src/babel subsume-json* cat test.js console.log("\ ".length) ✓ jridgewell@Dandy ~/src/babel subsume-json* nvm exec 0.12 node test.js Running node v0.12.10 (npm v2.14.9) 0 ✓ jridgewell@Dandy ~/src/babel subsume-json* nvm exec 10 node test.js Running node v10.1.0 (npm v5.6.0) 0

Ohh you're right, I looked at the wrong escape logic. This is the handling for cases like

var foo = "first\ second";

where the backslash escapes the newline, and those are treated as non-existent charaters. See https://www.ecma-international.org/ecma-262/8.0/#sec-static-semantics-sv

The SV of DoubleStringCharacter::LineContinuation is the empty code unit sequence.

and

The SV of SingleStringCharacter::LineContinuation is the empty code unit sequence.

This is indeed a LineContinuation. The proposal doesn’t change how "\<U+2028>" or "\<U+2029>" behave.

As a side note, please only ever link to https://tc39.github.io/ecma262/, not to the versioned snapshot on ecma-international.org.

@mathiasbynens I think it depends what you're linking. I generally purposefully link to the explicit versioned page because it means that links won't break over time if a section gets renamed or something. If it's something that's specifically landed since the last version, then totally, but that isn't the case here.

If you’re trying to avoid links that “break over time”, that’s all the more reason to link to the latest draft! The content in the link you posted was broken the moment a new commit was pushed to https://tc39.github.io/ecma262/ that was not reflected in the snapshot.

IDs are preserved even when section names change, so that’s not an issue. And in this PR we’re discussing something that hasn’t even landed in the draft spec yet, so there is no reason at all to link to an outdated spec.

The content in the link you posted was broken the moment

That's why I said it is dependent on what you're linking. I'm not trying to link to an ever-updating version of the document, because this specific discussion is about the current state of things, and I want it to stay pointing at the current state of things at the time of the discussion. This is the same reason I always link to files in git repos with SHA-specific links, rather than to master. If I could link to a specific SHA when linking to https://tc39.github.io/ecma262/ I'd happily do so, but I don't know if that's possible. If so, I'd be happy to do so. If my objective was to link to a persistent updating version of that section, I'd 100% agree with what you're saying, but that was not my intention in this discussion.

IDs are preserved even when section names change, so that’s not an issue.

That's good to know. I've never heard that as an official policy anywhere, do you have a link to support that?

And in this PR we’re discussing something that hasn’t even landed in the draft spec yet, so there is no reason at all to link to an outdated spec.

The text I quoted is unchanged from what I linked to in both the proposal and the current spec text, so it seems entirely my choice.

This is the same reason I always link to files in git repos with SHA-specific links, rather than to master.

This is an apples-to-oranges comparison. Linking to specific lines of code is different from linking to a spec.

When you link to the Babel repo itself, do you link to https://github.com/babel/babel or to https://github.com/babel/babel/tree/c558dedd7b5ffacf4d462177857cc031e7f4efe0? Why would it be different when linking to a spec?

That's good to know. I've never heard that as an official policy anywhere, do you have a link to support that?

It’s why we have es6num entries in the spec.

I'm not trying to link to an ever-updating version of the document, because this specific discussion is about the current state of things

The only document reflecting the current state of things is the draft. In this particular case, it’s the draft + the proposal. In this context especially, linking to an outdated snapshot doesn’t make sense.

Web standards are a moving target, and pretending otherwise is harmful. The WHATWG FAQ has a useful entry on this, which applies not just to their own standards, but to web standards in general: https://whatwg.org/faq#stable

This is an apples-to-oranges comparison. Linking to specific lines of code is different from linking to a spec.

I guess this is the primary point I disagree on. Any discussion has both a content, and a temporal aspect. If my intention was to answer "what section of the spec defines the string-value algorithm, I'd 100% agree that linking to https://tc39.github.io/ecma262/ with an ID alone would be the right call. But that's not what I was doing, I was quoting text from the spec at a specific point in time. That may change in the future, that's great, but I want to my link to stay pointing at the exact version that I took the quoted text from, because this discussion focuses on the spec as it is today, but as it will be in a year or whatever.

When you link to the Babel repo itself, do you link to babel/babel or to c558ded?

That would entirely depend on if I'm attempting to link to the repo in general, or the repo at a specific point in time.

It’s why we have es6num entries in the spec.

Awesome, I'm happy to hear that it is an official goal to keep IDs consistent.

Web standards are a moving target, and pretending otherwise is harmful. The WHATWG FAQ has a useful entry on this, which applies not just to their own standards, but to web standards in general: whatwg.org/faq#stable

I don't disagree, and I'm in no way pretending otherwise, but it was my only option because I can't provide a moment in time to view the spec on Github, and the most recent immutable version I could reach for was the published 8.0 document. I'd love to be able to link to https://tc39.github.io/ecma262/?SHA=ABC... to link to a specific moment in time. Then the spec page itself could trivially say "hey this section has been updated since the link was create" or something, but it doesn't currently do that.

The fact that specs are moving toward being living standards is fantastic and I fully support it, but it doesn't mean it isn't useful to be able to link to a spec at a specific moment.

babel-bot · 2018-05-19T01:08:57Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/8067/

loganfsmyth · 2018-05-19T02:00:57Z

packages/babylon/test/fixtures/experimental/json-strings/string-line-separator/input.js

@@ -1,3 +1,4 @@
-// break directive parsing
+breakDirectiveParsing;


Maybe we can do

("before after");

instead?

loganfsmyth

LGTM other than the README. I don't feel strongly about the breakDirectiveParsing; thing if you want to leave it.

loganfsmyth · 2018-05-19T02:01:56Z

packages/babel-plugin-proposal-json-strings/README.md

@@ -0,0 +1,60 @@
+# @babel/plugin-proposal-optional-catch-binding


Old README content.

loganfsmyth · 2018-05-19T02:16:52Z

.../babel-plugin-proposal-json-strings/test/fixtures/json-strings/string-line-separator/exec.js

+expect(" ".length).toBe(1);
+//      ^ That's a U+2028 LINE SEPARATOR UTF-16 char
+
+expect("\ ".length).toBe(0);


Huh, that seems like a bug. The proposal doesn't appear to change the behavior for escaped unicode characters, just unescaped ones, and JSON.parse("\\\u2028") throws, so this probably should too.

https://github.com/tc39/proposal-json-superset is at stage 3. This allows U+2028 LINE SEPARATOR and U+2029 PARAGRAPH SEPARATOR to appear unescaped inside strings and directives.

loganfsmyth · 2018-05-19T04:54:02Z

packages/babel-plugin-proposal-json-strings/src/index.js

+
+export default declare(api => {
+  api.assertVersion(7);
+  const regex = /(^|[^\\])([\u2028\u2029])/g;


This type of check with [^\\] is always a bit dangerous because it will fail on code like

"before\\ after";

since there is a slash beforehand, but it was already escaped and thus not an issue. Not super common, but it is an edge case that isn't handled here.

Good catch, fixed.

loganfsmyth · 2018-05-19T05:33:38Z

packages/babel-plugin-proposal-json-strings/src/index.js

+    // If there's an odd number, that means the separator itself was escaped.
+    // "\X" escapes X.
+    // "\\X" escapes the backslash, so X is unescaped.
+    const isEscaped = escapes.length % 2 === 1;


existentialism

Nice!

* Add Subsume JSON transform https://github.com/tc39/proposal-json-superset is at stage 3. This allows U+2028 LINE SEPARATOR and U+2029 PARAGRAPH SEPARATOR to appear unescaped inside strings and directives. * Move to Stage 3 * Break diretive parsing * Update README * Handle multi-escape sequences * Remove babylon files after rename

jridgewell added PR: Spec Compliance 👓 A type of pull request used for our changelog categories PR: New Feature 🚀 A type of pull request used for our changelog categories labels May 19, 2018

jridgewell commented May 19, 2018

View reviewed changes

babel deleted a comment from babel-bot May 19, 2018

loganfsmyth reviewed May 19, 2018

View reviewed changes

jridgewell added 4 commits May 19, 2018 00:06

Add Subsume JSON transform

1bef354

https://github.com/tc39/proposal-json-superset is at stage 3. This allows U+2028 LINE SEPARATOR and U+2029 PARAGRAPH SEPARATOR to appear unescaped inside strings and directives.

Move to Stage 3

60a34ee

Update README

cf8996c

Break diretive parsing

a4d8830

jridgewell force-pushed the subsume-json branch from d434acf to a4d8830 Compare May 19, 2018 04:06

loganfsmyth reviewed May 19, 2018

View reviewed changes

Handle multi-escape sequences

61638c9

loganfsmyth reviewed May 19, 2018

View reviewed changes

loganfsmyth approved these changes May 19, 2018

View reviewed changes

Remove babylon files after rename

7fde09b

existentialism approved these changes May 19, 2018

View reviewed changes

jridgewell merged commit 0879a6d into babel:master May 19, 2018

jridgewell deleted the subsume-json branch May 19, 2018 19:32

gibson042 mentioned this pull request May 21, 2018

Add tests for LineContinuation in StringLiteral tc39/test262#1565

Merged

hzoo mentioned this pull request May 22, 2018

Subsume JSON strings (Stage 4) babel/proposals#43

Open

loganfsmyth mentioned this pull request Oct 11, 2018

'jsonStrings' parser plugin does not increment line count #8865

Closed

lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 4, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 4, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Subsume json #7985

Subsume json #7985

jridgewell commented May 19, 2018

jridgewell May 19, 2018

loganfsmyth May 19, 2018

jridgewell May 19, 2018

loganfsmyth May 19, 2018

mathiasbynens May 21, 2018 •

edited

Loading

loganfsmyth May 21, 2018

mathiasbynens May 21, 2018

loganfsmyth May 21, 2018

mathiasbynens May 21, 2018

loganfsmyth May 21, 2018

babel-bot commented May 19, 2018 •

edited

Loading

loganfsmyth May 19, 2018

jridgewell May 19, 2018

loganfsmyth left a comment

loganfsmyth May 19, 2018

jridgewell May 19, 2018

loganfsmyth May 19, 2018

loganfsmyth May 19, 2018

jridgewell May 19, 2018

loganfsmyth May 19, 2018

existentialism left a comment

		@@ -1,3 +1,4 @@
		// break directive parsing
		breakDirectiveParsing;

		@@ -0,0 +1,60 @@
		# @babel/plugin-proposal-optional-catch-binding

Subsume json #7985

Subsume json #7985

Conversation

jridgewell commented May 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mathiasbynens May 21, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

babel-bot commented May 19, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loganfsmyth left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

existentialism left a comment

Choose a reason for hiding this comment

mathiasbynens May 21, 2018 •

edited

Loading

babel-bot commented May 19, 2018 •

edited

Loading