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

Generator function typed with non optional parameter, throws error TS2345 #31

Closed
theseyi opened this issue Jan 13, 2019 · 9 comments
Closed

Comments

@theseyi
Copy link

theseyi commented Jan 13, 2019

I'm seeing the error below when trying to annotate a generator function that takes a required parameter. If I make the parameter optional, then all is well. However, this would make my typings inaccurate!
If I define a class with the ffg attributes, I receive the commented out error from tsc.

It seems like a non-compliant type definitions, but my local investigation does not seem to back that up. Is this a known issue? How can I resolve this

export class EmberConcurrencyDecorators extends Controller {
  @task // Error: TS2345: Argument of type '"approve"' is not assignable to parameter of type '"model" | "_super" | undefined'.
  *approve(id: string) {
    yield id
  }


  @task // OK!
  *disapprove(id?: string) {
    yield id
  }
}
@simonihmig
Copy link

Seeing the same!

My dependencies (excerpt):

    "@ember-decorators/babel-transforms": "^3.1.0",
    "ember-cli-babel": "^7.2.0",
    "ember-cli-typescript": "^2.0.0-rc.2",
    "ember-decorators": "^5.0.1",
    "typescript": "^3.2.2",

@buschtoens
Copy link
Collaborator

Thanks for reporting and confirming. I added index.d.ts in an attempt to provide as much type safety as possible. But evidently TypeScript was smarter than me.

You're doing everything correctly. I'll try to revise the types and loosen them, if not possible.

@buschtoens buschtoens added the bug label Jan 16, 2019
@buschtoens
Copy link
Collaborator

Can you share your tsconfig.json? I am not able to reproduce this. 🤔

@simonihmig
Copy link

Sure:

{
  "compilerOptions": {
    "target": "es2017",
    "allowJs": true,
    "moduleResolution": "node",
    "allowSyntheticDefaultImports": true,
    "noImplicitAny": true,
    "noImplicitThis": true,
    "alwaysStrict": true,
    "strictNullChecks": true,
    "strictPropertyInitialization": true,
    "experimentalDecorators": true,
    "noFallthroughCasesInSwitch": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "noImplicitReturns": true,
    "noEmitOnError": false,
    "noEmit": true,
    "inlineSourceMap": true,
    "inlineSources": true,
    "baseUrl": ".",
    "module": "es6",
    "skipLibCheck": true,
    "paths": {
      "mpp-frontend/tests/*": [
        "tests/*"
      ],
      "mpp-frontend/mirage/*": [
        "mirage/*"
      ],
      "mpp-frontend/src/*": [
        "src/*"
      ],
      "*": [
        "types/*"
      ]
    }
  },
  "include": [
    "src/**/*",
    "tests/**/*",
    "types/**/*",
    "mirage/**/*"
  ]
}

@theseyi
Copy link
Author

theseyi commented Jan 16, 2019

in base.tsconfig

{
    "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,
      "module": "es6",
      "lib": ["es2017.object", "es2016", "dom", "dom.iterable", "scripthost"],
      "experimentalDecorators": true,
      "downlevelIteration": true
    },
    "exclude": ["tmp", "dist", "vendor", ".git", ".gradle", ".idea", "logs"]
  }

in extended.tsconfig

{
  "extends": "../tsconfig-base",
  "compilerOptions": {
    "baseUrl": ".",
    "alwaysStrict": false,
    "strictPropertyInitialization": false,
    "lib": ["es2017.object", "es2016", "dom", "dom.iterable", "scripthost"],
    "listEmittedFiles": false,
    "paths": {
      "app/*": ["app/*", "node_modules/<addon-app>"],
      "app/tests/*": ["tests/*"],
      "app/mirage/*": ["mirage/*"],
      "addon-app": ["node_modules/<addon-app>/addon"],
      "addon-app/*": ["node_modules/<addon-app>/addon/*"]
    }
  },
  "include": [
    "app/**/*",
    "tests/**/*",
    "mirage/**/*",
    "node_modules/<addon-app>/app",
    "node_modules/<addon-app>/addon"
  ]
}

@buschtoens
Copy link
Collaborator

buschtoens commented Jan 16, 2019

Thanks a lot for reporting and providing your configs!

I published a fix as 0.5.1 0.5.2. 🎉

Let me know, if it works for you. 😊

Edit: The latest version should also support encapsulated tasks now.

@buschtoens
Copy link
Collaborator

FYI you're gonna run into more type errors, when actually trying to perform the task. However, after weeks of trial and error (and despair 😅), I have found a potential solution: #30

It's definitely not perfect, but I think it provides the least amount of friction for the most use cases. I've posted it to #e-typescript as well, to get some export opinions on it.

@theseyi
Copy link
Author

theseyi commented Jan 17, 2019

Awesome! Thanks @buschtoens , I will try this over the weekend with my upgrade to ember3.6

@simonihmig
Copy link

Can confirm, TS error is gone! Thanks Jan for the super quick fix! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants