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

Babel decorator runtime helper errors for class with overloaded method #590

Closed
simonihmig opened this issue Feb 25, 2019 · 8 comments
Closed
Labels
bug build Ideas for or bugs with the build process

Comments

@simonihmig
Copy link
Contributor

Please paste the output of ember -v here

ember-cli: 3.7.1
node: 10.11.0
os: darwin x64

Please paste the output of tsc -v here

Version 3.3.3333

Please paste your tconfig.json and tslint.json or eslint.json (if applicable) below

My tsconfig.json

{
"compilerOptions": {
"target": "es2017",
"allowJs": true,
"moduleResolution": "node",
"allowSyntheticDefaultImports": true,
"noImplicitAny": true,
"noImplicitThis": true,
"alwaysStrict": true,
"strictNullChecks": true,
"strictPropertyInitialization": true,
"noFallthroughCasesInSwitch": true,
"noUnusedLocals": true,
"noUnusedParameters": true,
"noImplicitReturns": true,
"noEmitOnError": false,
"noEmit": true,
"inlineSourceMap": true,
"inlineSources": true,
"baseUrl": ".",
"module": "es6",
"experimentalDecorators": true,
"paths": {
"ember-typescript-overloads-decorators-bug/tests/": [
"tests/
"
],
"ember-typescript-overloads-decorators-bug/": [
"app/
"
],
"": [
"types/
"
]
}
},
"include": [
"app//*",
"tests/
/",
"types/**/
"
]
}

What are instructions we can follow to reproduce the issue?

git clone https://github.com/simonihmig/ember-typescript-overloads-decorators-bug
Reproduction Case

See the above reproduction repo.

Now about that bug. What did you expect to see?

Hit http://localhost:4200/ and see the default welcome page.

What happened instead?

Runtime error: Uncaught TypeError: Duplicated element (foo)

This happens when a TS class has at least one decorator and one overloaded method. Seems to be an issue with how the decorators and TS Babel plugins interact AFAICT.

The transpiled output of https://github.com/simonihmig/ember-typescript-overloads-decorators-bug/blob/master/app/controllers/application.ts looks like this:

let ApplicationController = (0, _decorate2.default)(null, function (_initialize, _EmberController) {
    class ApplicationController extends _EmberController {
      constructor(...args) {
        super(...args);

        _initialize(this);
      }

    }

    return {
      F: ApplicationController,
      d: [{
        kind: "field",
        decorators: [_service.inject],
        key: "router",
        value: void 0
      }, {
        kind: "field",
        key: "foo",
        value: void 0
      }, {
        kind: "field",
        key: "foo",
        value: void 0
      }, {
        kind: "method",
        key: "foo",
        value: function foo() {// dummy
        }
      }]
    };
  }, Ember.Controller);

So the overloaded foo method, which should affect only the typing, not the runtime code, causes multiple foo fields to be defined for the decorator runtime (@babel/runtime/helpers/esm/decorate), which then errors because of this.

@lolmaus
Copy link

lolmaus commented Mar 2, 2019

@chriskrycho
Copy link
Member

@simonihmig can you add the ember-cli-typescript version to the writeup there when you get a chance, please?

@chriskrycho chriskrycho added bug build Ideas for or bugs with the build process labels Mar 4, 2019
@chriskrycho
Copy link
Member

At first blush this definitely looks like a bug in the intersection of Babel's TS plugin and its class fields plugin… and it doesn't happen if decorators are present? Quite a specific corner if that's the case.

@simonihmig
Copy link
Contributor Author

can you add the ember-cli-typescript version to the writeup there when you get a chance, please?

Using 2.0.0-rc.2, same in the reproduction: https://github.com/simonihmig/ember-typescript-overloads-decorators-bug/blob/master/package.json#L45

and it doesn't happen if decorators are present

You probably mean the opposite, so it doesn't happen if decorators are not present. So both condition must be met: overloaded method + decorator used.

Without decorators you don't get this "massively" transpiled structure:

return {
      F: ApplicationController,
      d: [...]
}

Just a simple ES6 class, with the overloaded type definitions being removed from the final output.

It looks as if the decorator babel plugin runs first before the overloaded type definitions are stripped, and while generating this structure above it interprets the overloaded type definitions as separate methods (all with the same key!), puts them all into this d: [...] array, which causes the runtime decorator helper later to fail. If the TS plugin would have stripped the type definitions first, this would probably not happen. Just my naive interpretation though...

Maybe @pzuraq has some ideas?

@pzuraq
Copy link

pzuraq commented Mar 4, 2019

We're planning on walking back the decorators transforms to stage 1, so maybe try those transforms and see if it works better? @ember-decorators/babel-transforms@^2.1.0 should work

@pzuraq
Copy link

pzuraq commented Mar 4, 2019

See the latest revision of emberjs/rfcs#440 for more details

@lifeart
Copy link

lifeart commented Mar 14, 2019

Maybe related to ember-cli/ember-ajax#428

@chriskrycho
Copy link
Member

I believe this has been resolved upstream long since; but in any case the goal will be to align with spec decorators (and will not be something fixed in a TS-specific way).

@chriskrycho chriskrycho closed this as not planned Won't fix, can't repro, duplicate, stale Sep 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug build Ideas for or bugs with the build process
Projects
None yet
Development

No branches or pull requests

5 participants