Transformer Test Case Problems

[Boshen]

This discussion records all the problems we encountered while working on the transformer.

Each answer is for a specific problem and its matching test cases.

cc @overlookmotel @Dunqing

[Dunqing]

scope.hasOwnBinding()

Call from here

babel-plugin-transform-typescript

• imports/enum-value/input.ts

[overlookmotel]

Hmm I see the problem. I think you can do this with VisitMut, but you'd need to add visitors to keep track of what scope you're in. e.g. visit_function would push the scope_id of the function to a stack, so you can then read it back later. But this means re-implementing all the scope-tracking functionality that Traverse already has. This is possible, but a huge pain.

[overlookmotel]

Actually... could something like this work?

1. Add a flag is_in_enum to TypeScriptEnum.
2. Set that flag to true when entering an enum, and back to false when exiting it.
3. Add a visitor for enter_identifier_reference to TypeScriptEnum.
4. In enter_identifier_reference, it checks if is_in_enum is true, and if it is then it reads ctx.scopes() and can decide whether to replace the identifier or not.

i.e. Do everything in one Traverse pass, instead of traverse-within-traverse.

I think this could work? But maybe I missed something?

[Dunqing]

Sounds good. I've thought it through. The only problem is previous_enum_members. In each transform enum member. We will cache the value of enum member to previous_enum_members. The reason for doing this is because the initializer of an enum member can use the enum members that have already been defined before it. So we may not solve this case.

[overlookmotel]

I guess it's possible to have an enum within an enum.

enum A {
  a = 1,
  b = function() {
    enum B {
      c = a,
      d = x,
    }
  }
}

so instead of a single flag is_in_enum, would need a stack of "current enums", to support this bizarre use case.

So we may not solve this case.

I don't see the problem with that one. What's the catch? If no binding found in upper scope, it defaults to resolving foo as Enum.foo. So, in that example, is it a problem that d is used before it's defined?

[Dunqing]

Here is the entry point for the transform enum.

oxc/crates/oxc_transformer/src/typescript/mod.rs

Lines 140 to 147 in 3c55da6

	let new_stmt = match stmt {
	match_declaration!(Statement) => {
	if let Declaration::TSEnumDeclaration(ts_enum_decl) = &stmt.to_declaration() {
	self.r#enum.transform_ts_enum(ts_enum_decl, false, ctx)
	} else {
	None
	}
	}

This means we start traversing the new statement only after transforming the whole enum.

I've thought it over carefully, and it's getting a bit complicated. Because the value of an enum member can be any Expression. If we want to modify the identifier reference after transforming the enum, we may encounter many edge-case

[Dunqing]

scope.generateUid()

Call from here

babel-plugin-transform-typescript

• namespace/canonical/input.ts
• namespace/contentious-names/input.ts
• namespace/empty-removed/input.ts
• namespace/module-nested/input.ts
• namespace/module-nested-export/input.ts
• namespace/multiple/input.ts
• namespace/mutable-fail/input.ts
• namespace/namespace-flag/input.ts
• namespace/namespace-nested-module/input.ts
• namespace/nested/input.ts
• namespace/nested-destructuring/input.ts
• namespace/nested-namespace/input.ts
• namespace/nested-shorthand-export/input.ts
• namespace/same-name/input.ts

babel-plugin-transform-react-jsx

• autoImport/complicated-scope-module/input.js

[overlookmotel]

#3314 added all the primitives required to support this. I will build an abstraction around them to produce a generateUid-type API next week.

[overlookmotel]

Please see #3394 and #3395.

[overlookmotel]

@Dunqing As you mentioned in #3395 (comment), it sounds like you're going to tackle the rest of these, now that TraverseCtx::generate_uid is implemented.

But please can you leave the React JSX one for me? I would like to get some practice on the transformer, and it would be easier for me to do that on a JSX transform (where I understand what the transform is doing), rather than TypeScript ones (where I don't understand, because I don't know TS).

[Dunqing]

Sure, enjoy it.

[overlookmotel]

#3431 solves the React JSX one.

[Dunqing]

Complicated cases

babel-plugin-transform-typescript

• enum/mix-references/input.ts
• enum/ts5.0-const-foldable/input.ts

These tests require the support of these API

• scope.hasOwnBinding - call from here
• scope.getBindingIdentifier() - call from here
• path.resolve() - call from here

[overlookmotel]

scope.hasOwnBinding and scope.getBindingIdentifier equivalents are now available via the APIs added in #3314.

Not sure about path.resolve - I've not investigated what it does yet.

[Dunqing]

What is the scope.getBindingIdentifier equivalent api?

path.resolve() is to find the defined node based on the current node. For example, if the current node is an IdentifierReference. The return value of path.resolve is a BindingIdentifier

let a = 0;
    ^
    The result of path.resolve() 
console.log(a) // call path.resolve() here

[Dunqing]

ScopeFlags

babel-plugin-transform-react-jsx-self

• react-source/disable-with-super/input.js

babel-plugin-transform-react-jsx-development

• cross-platform/within-derived-classes-constructor/input.js

These tests need to know what the current node's ScopeFlags are

[overlookmotel]

This one has been addressed in #3229. ctx.scope() gives you ScopeFlags for the current node.

Please see notes in that PR about inheritance of flags from parent scope.

[Dunqing]

Thank you! These tests were done.

[Dunqing]

babel-plugin-transform-react-jsx-development

• cross-platform/within-ts-module-block/input.ts

This test needs to enable typescript in codegen and turn off the typescript plugin

[Boshen]

I'll follow up on this.

[Boshen]

Need to add an option to disable typescript plugin.

[Dunqing]

We need to port registerDeclaration api. In the namespace, we will transform the namespace {} to function() {}. But, currently, the new function has no scope_id. We need to create a scope and bind it to a new function. Currently, block this case namespace/empty-removed/input.ts

[Dunqing]

@overlookmotel cc

[overlookmotel]

If I understand how namespace {} works, TSModuleBlock represents the {} part and it has a scope_id.

The transform removes the TSModuleBlock, and replaces with a function. So can we "steal" the scope_id from the old TSModuleBlock and make it the scope_id for the new function instead?

[Dunqing]

Yes, It's a workaround. I just wanted to let you know that we can do it for now. Their scope flags are different. We may also need to provide a method to change it.

[Dunqing]

I have done in #3459 #3460. All namespace test cases are done!

[overlookmotel]

All namespace test cases are done!

🍾 Bravo Dunqing!

I will review those 2 PRs later today.

Yes, we definitely need to add new APIs for manipulating scopes, symbols + references.

In this particular case, we should be able to alter the scope flags for the old TSModuleBlock scope via ctx.scopes_mut().

[overlookmotel]

Do we need to transform UsingDeclaration for this milestone?

As #3482 points out, it's not exclusively TS syntax, but as it's currently a stage 3 proposal it will likely need to be transpiled to run in browsers/NodeJS. caniuse.com doesn't list it, which I assume means it's not implemented in browsers yet.

[Boshen]

It's not part of the current milestone.