Type strip and transform experimental TS decorators without renaming declarations?

[dsherret]

It seems in the latest version of swc_ecmascript the hygiene() pass will rename many TS ast names. For example, something like:

namespace Test {
  export const t = 5;
}

...will be renamed to Test1. The problem is that when removing hygiene(), decorators don't work. For example, something like:

function A() {
  console.log("@A evaluated");
  return function (
    target: any,
    propertyKey: string,
    descriptor: PropertyDescriptor,
  ) {
    console.log("@A called");
    const fn = descriptor.value;
    descriptor.value = function () {
      console.log("fn() called from @A");
      fn();
    };
  };
}

function B() {
  console.log("@B evaluated");
  return function (
    target: any,
    propertyKey: string,
    descriptor: PropertyDescriptor,
  ) {
    console.log("@B called");
    const fn = descriptor.value;
    descriptor.value = function () {
      console.log("fn() called from @B");
      fn();
    };
  };
}

class C {
  @A()
  @B()
  static test() {
    console.log("C.test() called");
  }
}

C.test();

Will have some output that looks like this:

var _class,_dec,_dec;

...which is resolved by doing a hygiene() pass.

I noticed that names are not renamed on the swc playground, but once setting "decorators": true under TypeScript and trying out the example above, it will have the bug of outputting var _class,_dec,_dec... so looks like it is not doing a hygiene() pass.

Is this something that swc could support or is this just a limitation of it? If so, I can work around it, but I was wondering if it's desired in swc or if there is a way to do this. Maybe the fix might be to improve the emit of experimental decorators.

Thanks!

[kdy1]

The problem here is the ownership model of rust.
We have a way to handle scoping. But scoping is very hard in rust. We need to ensure that Scope is not in an invalid state.

To do so, we need mutable references to AST and the scope at the same time, and we have to ensure we update Scope every time we update AST.

We may add Scope to AST and mutate it using methods, but then we have to determine which node should store it. If it's stored in Module, we now have to find a way to update it when Ident is modified. If we try to store it in Idents, we have to store Arc<Mutext<Scope>> in all Idents.

That's why I implemented identifier management based on hygiene information of Span. Scope is managed by only two passes, which are resolver and hygiene. By using this, we can avoid updating the scope completely.

I think hygiene can be modified to preserve original identifiers by sharing its state with resolver, but I'm not sure if it will work.

[dsherret]

It seems like a file parsed and transformed as a script containing a namespace like namespace Test { export let t = 1; } will also be changed to Test1. I believe that's a bug because Test is global in that case? Edit: opened #2808

[kdy1]

I think about it more, and marking identifiers as 'should-be-preserved' will work.
Problem is the bundler, which mixes identifiers from various modules.

/cc @dsherret
/cc @bartlomieju
Is preserving identifiers for normal files enough? (Except bundler)

[bartlomieju]

I'm not 100% sure, but I think there are also some issues where globals are being renamed in bundler which causes some bugs as well, but I'll defer to @dsherret for more concrete answer.

[dsherret]

I think it's ok for bundling when using modules. The only issue might be when bundling scripts, but Deno is not affected by that.

On a side note, in this scenario, I feel like there must be a transform for type stripping enums and namespaces that gives the identifier the same syntax context so it doesn't add a 1 and acts like other declarations like classes. I think right now it does this because it emits a var and that could cause issues. Why not emit let and keep the declaration as-is so it doesn't add a 1? Seems like that would work, but I bet there's some reason it does var right now. A transform down to ES5 would then change the let to a var. Edit: nope... declaration merging. Actually using let should work for enums, but not namespaces.

[dsherret]

Oh, I completely forgot about declaration merging.

[dsherret]

Resolved by #2940.