Skip to content

Commit

Permalink
Fix nullish coalescing when the RHS is an object literal (#516)
Browse files Browse the repository at this point in the history
Fixes #505

The RHS is meant to be transpiled into an expression arrow function, but if the
expression is an object literal, it gets interpreted as a block body. To fix, we
can unconditionally surround the arrow body in parens. This is a little ugly in
that it leaves an extra space, but avoiding that is probably more trouble than
it's worth.

Also fix two issues with test infra:
* The build was only running the very last build configuration due to what seems to be a
  breaking change in Travis. Updated the yaml file to use `jobs` instead of `matrix`, which
  seems to work better.
* Three Babel tests had broken due to fixture tests that were testing unpinned behavior (looks
  like regex transformation and preset-env behavior). I was hoping to fix them by pinning the
  dependencies or updating to latest Babel, but ended up just updating the expected test code
  for now. I confirmed that this was a regression on the Babel end rather than anything caused by
  a Sucrase change.
  • Loading branch information
alangpierce authored Mar 22, 2020
1 parent c274a7e commit 0d91fd2
Show file tree
Hide file tree
Showing 5 changed files with 72 additions and 16 deletions.
19 changes: 9 additions & 10 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
language: node_js
node_js:
- '12'
# Theoretically we could cache node_modules and example-repos, but those end up
# being very slow to download and are largely duplicated with yarn cache.
cache:
Expand All @@ -11,19 +9,23 @@ branches:
- "master"
before_install:
- npm install -g yarn@latest
matrix:
jobs:
include:
# Put the slowest builds first, since they are more likely to start first
- script:
- node_js: '12'
script:
- yarn fast-build
- yarn run-examples babel
- script:
- node_js: '12'
script:
- yarn fast-build
- yarn run-examples react decaffeinate decaffeinate-parser coffee-lex
- script:
- node_js: '12'
script:
- yarn fast-build
- yarn run-examples tslint apollo-client
- script:
- node_js: '12'
script:
- yarn build
- yarn lint
- yarn test-with-coverage && yarn report-coverage
Expand All @@ -32,6 +34,3 @@ matrix:
script:
- yarn build
- yarn test-only
# Exclude the default build; we only want to run explicitly-included builds.
exclude:
node_js: '12'
34 changes: 34 additions & 0 deletions example-runner/example-configs/babel.patch
Original file line number Diff line number Diff line change
Expand Up @@ -174,6 +174,40 @@ index d92699883..2bdedecaa 100644

export type BindingTypes =
| typeof BIND_NONE
diff --git a/packages/babel-plugin-transform-classes/test/fixtures/regression/T6755/output.js b/packages/babel-plugin-transform-classes/test/fixtures/regression/T6755/output.js
index 204120c62..1950ebc1c 100644
--- a/packages/babel-plugin-transform-classes/test/fixtures/regression/T6755/output.js
+++ b/packages/babel-plugin-transform-classes/test/fixtures/regression/T6755/output.js
@@ -20,7 +20,7 @@ function () {
return _context.stop();
}
}
- });
+ }, null, null, null, Promise);
};

_proto.test2 =
diff --git a/packages/babel-plugin-transform-dotall-regex/test/fixtures/dotall-regex/with-unicode-property-escape/output.js b/packages/babel-plugin-transform-dotall-regex/test/fixtures/dotall-regex/with-unicode-property-escape/output.js
index a8ddf757d..c00aa15a9 100644
--- a/packages/babel-plugin-transform-dotall-regex/test/fixtures/dotall-regex/with-unicode-property-escape/output.js
+++ b/packages/babel-plugin-transform-dotall-regex/test/fixtures/dotall-regex/with-unicode-property-escape/output.js
@@ -1,2 +1,2 @@
-var a = /[\u3400-\u4DB5\u4E00-\u9FEF\uFA0E\uFA0F\uFA11\uFA13\uFA14\uFA1F\uFA21\uFA23\uFA24\uFA27-\uFA29\u{20000}-\u{2A6D6}\u{2A700}-\u{2B734}\u{2B740}-\u{2B81D}\u{2B820}-\u{2CEA1}\u{2CEB0}-\u{2EBE0}][\0-\t\x0B\f\x0E-\u2027\u202A-\u{10FFFF}]/u;
-var b = /[\u3400-\u4DB5\u4E00-\u9FEF\uFA0E\uFA0F\uFA11\uFA13\uFA14\uFA1F\uFA21\uFA23\uFA24\uFA27-\uFA29\u{20000}-\u{2A6D6}\u{2A700}-\u{2B734}\u{2B740}-\u{2B81D}\u{2B820}-\u{2CEA1}\u{2CEB0}-\u{2EBE0}][\0-\u{10FFFF}]/u;
+var a = /[\u3400-\u4DBF\u4E00-\u9FFC\uFA0E\uFA0F\uFA11\uFA13\uFA14\uFA1F\uFA21\uFA23\uFA24\uFA27-\uFA29\u{20000}-\u{2A6DD}\u{2A700}-\u{2B734}\u{2B740}-\u{2B81D}\u{2B820}-\u{2CEA1}\u{2CEB0}-\u{2EBE0}\u{30000}-\u{3134A}][\0-\t\x0B\f\x0E-\u2027\u202A-\u{10FFFF}]/u;
+var b = /[\u3400-\u4DBF\u4E00-\u9FFC\uFA0E\uFA0F\uFA11\uFA13\uFA14\uFA1F\uFA21\uFA23\uFA24\uFA27-\uFA29\u{20000}-\u{2A6DD}\u{2A700}-\u{2B734}\u{2B740}-\u{2B81D}\u{2B820}-\u{2CEA1}\u{2CEB0}-\u{2EBE0}\u{30000}-\u{3134A}][\0-\u{10FFFF}]/u;
diff --git a/packages/babel-plugin-transform-regenerator/test/fixtures/regression/4219/output.js b/packages/babel-plugin-transform-regenerator/test/fixtures/regression/4219/output.js
index a3b8fd1f2..3fa934c71 100644
--- a/packages/babel-plugin-transform-regenerator/test/fixtures/regression/4219/output.js
+++ b/packages/babel-plugin-transform-regenerator/test/fixtures/regression/4219/output.js
@@ -12,6 +12,6 @@ function test(fn) {
return _context.stop();
}
}
- });
+ }, null, null, null, Promise);
};
}
diff --git a/packages/babel-polyfill/src/index.js b/packages/babel-polyfill/src/index.js
index aa1b4639f..cca92db5a 100644
--- a/packages/babel-polyfill/src/index.js
Expand Down
2 changes: 1 addition & 1 deletion src/TokenProcessor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,7 @@ export default class TokenProcessor {
}
if (token.numNullishCoalesceEnds) {
for (let i = 0; i < token.numNullishCoalesceEnds; i++) {
this.resultCode += ")";
this.resultCode += "))";
}
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/transformers/OptionalChainingNullishTransformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ export default class OptionalChainingNullishTransformer extends Transformer {
if (this.tokens.matches1(tt.nullishCoalescing)) {
const token = this.tokens.currentToken();
if (this.tokens.tokens[token.nullishStartIndex!].isAsyncOperation) {
this.tokens.replaceTokenTrimmingLeftWhitespace(", async () =>");
this.tokens.replaceTokenTrimmingLeftWhitespace(", async () => (");
} else {
this.tokens.replaceTokenTrimmingLeftWhitespace(", () =>");
this.tokens.replaceTokenTrimmingLeftWhitespace(", () => (");
}
return true;
}
Expand Down
29 changes: 26 additions & 3 deletions test/sucrase-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -889,7 +889,7 @@ describe("sucrase", () => {
const x = a ?? b;
`,
`${NULLISH_COALESCE_PREFIX}
const x = _nullishCoalesce(a, () => b);
const x = _nullishCoalesce(a, () => ( b));
`,
{transforms: []},
);
Expand All @@ -901,7 +901,7 @@ describe("sucrase", () => {
const x = a ?? b ?? c;
`,
`${NULLISH_COALESCE_PREFIX}
const x = _nullishCoalesce(_nullishCoalesce(a, () => b), () => c);
const x = _nullishCoalesce(_nullishCoalesce(a, () => ( b)), () => ( c));
`,
{transforms: []},
);
Expand Down Expand Up @@ -961,6 +961,29 @@ describe("sucrase", () => {
);
});

it("correctly transpiles nullish coalescing with an object right-hand side", () => {
assertResult(
`
null ?? {x: 5}
`,
`${NULLISH_COALESCE_PREFIX}
_nullishCoalesce(null, () => ( {x: 5}))
`,
{transforms: []},
);
});

it("correctly handles nullish coalescing with an object on the right-hand side", () => {
assertOutput(
`
const value = null ?? {x: 5};
setOutput(value.x)
`,
5,
{transforms: []},
);
});

it("specifies the proper this when using optional chaining on a function call", () => {
assertOutput(
`
Expand Down Expand Up @@ -1086,7 +1109,7 @@ describe("sucrase", () => {
`,
`${ASYNC_NULLISH_COALESCE_PREFIX}
async function foo() {
return await _asyncNullishCoalesce(a, async () => await b());
return await _asyncNullishCoalesce(a, async () => ( await b()));
}
`,
{transforms: []},
Expand Down

0 comments on commit 0d91fd2

Please sign in to comment.