Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Typescript parameter decorators use incorrect scope #2147

Closed
kanoshin opened this issue Mar 31, 2022 · 2 comments
Closed

Typescript parameter decorators use incorrect scope #2147

kanoshin opened this issue Mar 31, 2022 · 2 comments

Comments

@kanoshin
Copy link

kanoshin commented Mar 31, 2022

Example TS code:

import { inject } from 'inject';
import { Abc } from './Abc';

class Root1 {
    constructor(@inject(Abc) private Abc: Abc) {}
}

class Root2 {
    constructor(@inject(Abc) private abc: Abc) {}
}

Example esbuild output for esbuild src/index.ts --outfile=dist/bundle.js command:

var __defProp = Object.defineProperty;
var __getOwnPropDesc = Object.getOwnPropertyDescriptor;
var __decorateClass = (decorators, target, key, kind) => {
  var result = kind > 1 ? void 0 : kind ? __getOwnPropDesc(target, key) : target;
  for (var i = decorators.length - 1, decorator; i >= 0; i--)
    if (decorator = decorators[i])
      result = (kind ? decorator(target, key, result) : decorator(result)) || result;
  if (kind && result)
    __defProp(target, key, result);
  return result;
};
var __decorateParam = (index, decorator) => (target, key) => decorator(target, key, index);
import { inject } from "inject";
import { Abc } from "./Abc";
let Root1 = class {
  constructor(Abc2) {
    this.Abc = Abc2;
  }
};
Root1 = __decorateClass([
  __decorateParam(0, inject(Abc2))
], Root1);
let Root2 = class {
  constructor(abc) {
    this.abc = abc;
  }
};
Root2 = __decorateClass([
  __decorateParam(0, inject(Abc))
], Root2);

When running the output code there will be the error: Abc2 is not defined. Notice that esbuild thinks that in Root1 case decorator parameter is equal to constructor parameter. Same thing works properly when decorator parameter named differently from constructor parameter.

The same code will transpile to JS properly when using TSC.

@evanw
Copy link
Owner

evanw commented Apr 1, 2022

Oh, that's strange. So the scope lookup for parameter decorators is supposed to happen at the top-level, not where the decorator is. I guess that makes sense based on where the generated code for the decorator ends up. I'll have to rework some things to fix this.

@evanw
Copy link
Owner

evanw commented Apr 1, 2022

I believe the relevant code in the TypeScript compiler is this code in src/compiler/checker.ts:

case SyntaxKind.Decorator:
    // Decorators are resolved at the class declaration. Resolving at the parameter
    // or member would result in looking up locals in the method.
    //
    //   function y() {}
    //   class C {
    //       method(@y x, y) {} // <-- decorator y should be resolved at the class declaration, not the parameter.
    //   }
    //
    if (location.parent && location.parent.kind === SyntaxKind.Parameter) {
        location = location.parent;
    }
    //
    //   function y() {}
    //   class C {
    //       @y method(x, y) {} // <-- decorator y should be resolved at the class declaration, not the method.
    //   }
    //

    // class Decorators are resolved outside of the class to avoid referencing type parameters of that class.
    //
    //   type T = number;
    //   declare function y(x: T): any;
    //   @param(1 as T) // <-- T should resolve to the type alias outside of class C
    //   class C<T> {}
    if (location.parent && (isClassElement(location.parent) || location.parent.kind === SyntaxKind.ClassDeclaration)) {
        location = location.parent;
    }
    break;

@evanw evanw closed this as completed in 1e301eb Apr 1, 2022
@kanoshin kanoshin changed the title Typescript import name collision with decorated parameter Typescript parameter decorators use incorrect scope Apr 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants