@samitouri / QOS-React-2 / commits / b9275f65bc

[ez] Add TODO bailout on non-backward compatible holey arrays

--- (pasted from comment) Instead of handling holey arrays, bail out with a TODO error. Older versions of babel seem to have inconsistent handling of holey arrays, at least when paired with HermesParser. When using these versions, we should bail out instead of throwing a Babel validation error. Issue: The babel ast definition for array elements changed from Array<PatternLike> to Array<PatternLike | null>. Older versions do not expect null in the ArrayPattern ast and will throw a validation error during Codegen. - HermesParser will parse [, b] into [NodePath<null>, NodePath<Identifier>] - Forget will try to preserve this holey array when we codegen back to js (e.g. we call a babel builder function arrayPattern([null, identifier])) - Babel will fail with `TypeError: Property elements[0] of ArrayPattern expected node to be of a type ["PatternLike"] but instead got null` PR that changed the AST definition: https://github.com/babel/babel/pull/10917/files#diff-19b555d2f3904c206af406540d9df200b1e16befedb83ff39ebfcbd876f7fa8aL52-R56 [ez] Add TODO bailout on non-backward compatible holey arrays --- (pasted from comment) Instead of handling holey arrays, bail out with a TODO error. Older versions of babel seem to have inconsistent handling of holey arrays, at least when paired with HermesParser. When using these versions, we should bail out instead of throwing a Babel validation error. Issue: The babel ast definition for array elements changed from Array<PatternLike> to Array<PatternLike | null>. Older versions do not expect null in the ArrayPattern ast and will throw a validation error during Codegen. - HermesParser will parse [, b] into [NodePath<null>, NodePath<Identifier>] - Forget will try to preserve this holey array when we codegen back to js (e.g. we call a babel builder function arrayPattern([null, identifier])) - Babel will fail with `TypeError: Property elements[0] of ArrayPattern expected node to be of a type ["PatternLike"] but instead got null` PR that changed the AST definition: https://github.com/babel/babel/pull/10917/files#diff-19b555d2f3904c206af406540d9df200b1e16befedb83ff39ebfcbd876f7fa8aL52-R56

Mofei Zhang committed Sep 21, 2023 at 20:54 UTC b9275f65bc77824b4f938637a8c4f972a5e21e21

5 files changed +64

compiler/packages/babel-plugin-react-forget/src/HIR/BuildHIR.ts

@@ -1385,6 +1385,14 @@ function lowerExpression(

1385 elements.push({

1386 kind: "Hole",

1387 });

1388 + if (builder.environment.bailoutOnHoleyArrays) {

1389 + builder.errors.push({

1390 + reason: `(BuildHIR::lower) Fix babel holey array backward compatibility.`,

1391 + severity: ErrorSeverity.Todo,

1392 + loc: expr.node.loc ?? null,

1393 + suggestions: null,

1394 + });

1395 + }

1396 continue;

1397 } else if (element.isExpression()) {

1398 elements.push(lowerExpressionToTemporary(builder, element));

compiler/packages/babel-plugin-react-forget/src/HIR/Environment.ts

+24

@@ -214,6 +214,28 @@ export type EnvironmentConfig = Partial<{

214 * Defaults to false

215 */

216 assertValidMutableRanges: boolean;

217 +

218 + /**

219 + * Instead of handling holey arrays, bail out with a TODO error.

220 + *

221 + * Older versions of babel seem to have inconsistent handling of holey arrays,

222 + * at least when paired with HermesParser. When using these versions, we should

223 + * bail out instead of throwing a Babel validation error.

224 +

225 + * The babel ast definition for array elements changed from Array<PatternLike>

226 + * to Array<PatternLike | null>. Older versions does not expect null in the

227 + * ArrayPattern ast and will throw a validation error.

228 + *

229 + * - HermesParser will parse [, b] into [NodePath<null>, NodePath<Identifier>]

230 + * - Forget will try to preserve this holey array when we codegen back to js

231 + * (e.g. we call a babel builder function arrayPattern([null, identifier]))

232 + * - Babel will fail with `TypeError: Property elements[0] of ArrayPattern

233 + * expected node to be of a type ["PatternLike"] but instead got null`

234 + *

235 + * PR that changed the AST definition

236 + * https://github.com/babel/babel/pull/10917/files#diff-19b555d2f3904c206af406540d9df200b1e16befedb83ff39ebfcbd876f7fa8aL52-R56

237 + */

238 + bailoutOnHoleyArrays: boolean;

239 }>;

240

241 export class Environment {

@@ -232,6 +254,7 @@ export class Environment {

254 disableAllMemoization: boolean;

255 enableEmitFreeze: ExternalFunction | null;

256 assertValidMutableRanges: boolean;

257 + bailoutOnHoleyArrays: boolean;

258 enableForest: boolean;

259

260 #contextIdentifiers: Set<t.Identifier>;

@@ -287,6 +310,7 @@ export class Environment {

310 this.assertValidMutableRanges = config?.assertValidMutableRanges ?? false;

311 this.validateNoSetStateInRender =

312 config?.validateNoSetStateInRender ?? false;

313 + this.bailoutOnHoleyArrays = config?.bailoutOnHoleyArrays ?? false;

314 this.enableForest = config?.enableForest ?? false;

315

316 this.#contextIdentifiers = contextIdentifiers;

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.backwards-compatible-holey-arrays.expect.md new

+20

@@ -0,0 +1,20 @@

1 +

2 +## Input

3 +

4 +```javascript

5 +// @bailoutOnHoleyArrays

6 +

7 +function Component() {

8 + return [1, , 3];

9 +}

10 +

11 +```

12 +

13 +

14 +## Error

15 +

16 +```

17 +[ReactForget] Todo: (BuildHIR::lower) Fix babel holey array backward compatibility. (4:4)

18 +```

19 +

20 +

\ No newline at end of file

compiler/packages/babel-plugin-react-forget/src/__tests__/fixtures/compiler/error.backwards-compatible-holey-arrays.js new

@@ -0,0 +1,5 @@

1 +// @bailoutOnHoleyArrays

2 +

3 +function Component() {

4 + return [1, , 3];

5 +}

compiler/packages/fixture-test-utils/src/compiler-utils.ts

@@ -29,6 +29,7 @@ export function transformFixtureInput(

29 let compilationMode: CompilationMode = "all";

30 let enableForest = false;

31 let enableNoAliasOptimizations = false;

32 + let bailoutOnHoleyArrays = false;

34 if (firstLine.indexOf("@compilationMode(annotation)") !== -1) {

35 assert(

@@ -88,6 +89,11 @@ export function transformFixtureInput(

89 enableNoAliasOptimizations = true;

90 }

92 + if (firstLine.includes("@bailoutOnHoleyArrays")) {

93 + bailoutOnHoleyArrays = true;

94 + }

95 +

96 +

97 return pluginFn(

98 input,

99 basename,

@@ -136,6 +142,7 @@ export function transformFixtureInput(

142 validateNoSetStateInRender,

143 enableEmitFreeze,

144 assertValidMutableRanges: true,

145 + bailoutOnHoleyArrays,

146 enableForest,

147 },

148 compilationMode,