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

Type errors when using TypeScript v3.5.1 #946

Closed
josemarluedke opened this issue May 29, 2019 · 9 comments
Closed

Type errors when using TypeScript v3.5.1 #946

josemarluedke opened this issue May 29, 2019 · 9 comments

Comments

@josemarluedke
Copy link
Contributor

I started having the following TypeScript type errors when upgraded to v3.5.1.

Note: This is an Octane app (Ember + Glimmer components).

node_modules/@glimmer/runtime/dist/types/lib/component/curried-component.d.ts:6:87 - error TS2677: A type predicate's type must be assignable to its parameter's type.
  Type 'CurriedComponentDefinition' is not assignable to type 'Maybe<Dict<unknown>>'.
    Type 'CurriedComponentDefinition' is not assignable to type 'Dict<unknown>'.
      Index signature is missing in type 'CurriedComponentDefinition'.

6 export declare function isComponentDefinition(definition: Maybe<Dict>): definition is CurriedComponentDefinition;
                                                                                        ~~~~~~~~~~~~~~~~~~~~~~~~~~

node_modules/@glimmer/util/dist/types/lib/destroy.d.ts:3:69 - error TS2677: A type predicate's type must be assignable to its parameter's type.
  Type 'SymbolDestroyable' is not assignable to type 'Maybe<Dict<unknown>>'.
    Type 'SymbolDestroyable' is not assignable to type 'Dict<unknown>'.
      Index signature is missing in type 'SymbolDestroyable'.

3 export declare function isDestroyable(value: Maybe<Dict>): value is SymbolDestroyable;
@chancancode
Copy link
Contributor

Hmm, why is your typescript building glimmer-vm?

@josemarluedke
Copy link
Contributor Author

My app is built using TypeScript (ember-cli-typescript).

Here is my tsconfig.json for context.

Note that path on glimmer/component is to solve issues with types (glimmerjs/glimmer.js#179).

{
  "compilerOptions": {
    "target": "ES2017",
    "allowJs": true,
    "allowSyntheticDefaultImports": true,
    "alwaysStrict": true,
    "baseUrl": ".",
    "experimentalDecorators": true,
    "inlineSourceMap": true,
    "inlineSources": true,
    "module": "ESNext",
    "moduleResolution": "node",
    "noEmit": true,
    "noEmitOnError": false,
    "noFallthroughCasesInSwitch": true,
    "noImplicitAny": true,
    "noImplicitReturns": true,
    "noImplicitThis": true,
    "noUnusedLocals": true,
    "noUnusedParameters": true,
    "strictNullChecks": true,
    "strictPropertyInitialization": true,
    "lib": ["es5", "es6", "es7", "dom", "esnext.asynciterable"],
    "paths": {
      "demand-web/tests/*": [
        "tests/*"
      ],
      "demand-web/*": [
        "app/*"
      ],
      "*": [
        "types/*"
      ],
      "fetch": [
        "node_modules/ember-fetch"
      ],
      "@glimmer/component": [
        "node_modules/@glimmer/component/dist/types/addon/-private/component.d.ts"
      ]
    },
    "types": [
      "ember",
      "ember-qunit",
      "ember-test-helpers",
      "ember-testing-helpers",
      "ember__test-helpers",
      "qunit",
      "rsvp"
    ]
  },
  "include": [
    "app/**/*",
    "tests/**/*",
    "types/**/*"
  ]
}

@josemarluedke
Copy link
Contributor Author

Just tried TypeScript v3.6.2 and still seeing these issues above.

@dfreeman
Copy link
Contributor

Hmm, why is your typescript building glimmer-vm?

@chancancode TypeScript's default behavior is to validate the typings included in any libraries you import as well—after all, it needs that information to know whether your own project's code is correct.

Downstream consumers can squash the error messages by setting skipLibCheck in their tsconfig.json, but that means they'll only be getting partial type information from the library. In general, things that would produce those errors end up getting typed as any, so disabling the messages ends up hiding the fact that you're effectively programming in plain JavaScript at those boundaries and hoping for the best.

@rwjblue I don't want to put words in your mouth, but my understanding is that this kind of breaking change in the TS compiler across minor releases is (super reasonably) a big part of your hesitation around shipping types with core packages?

Copy link
Member

rwjblue commented Sep 2, 2019

@rwjblue I don't want to put words in your mouth, but my understanding is that this kind of breaking change in the TS compiler across minor releases is (super reasonably) a big part of your hesitation around shipping types with core packages?

Exactly.

chriskrycho added a commit to lifeart/ember-language-server that referenced this issue Sep 20, 2019
Updating the compiler version resolves some major performance problems
triggered by the Glimmer type definitions. Unfortunately, it also causes
some noise due to [glimmerjs/glimmer-vm#946], but that should be fixed
in an upcoming release of the `@glimmer/*` packages, and it does not
cause internal issues for this package.

[glimmerjs/glimmer-vm#946]: glimmerjs/glimmer-vm#946
chriskrycho added a commit to lifeart/ember-language-server that referenced this issue Sep 20, 2019
Updating the compiler version resolves some major performance problems
triggered by the Glimmer type definitions. Unfortunately, it also causes
some noise due to [glimmerjs/glimmer-vm#946], but that should be fixed
in an upcoming release of the `@glimmer/*` packages, and it does not
cause internal issues for this package.

[glimmerjs/glimmer-vm#946]: glimmerjs/glimmer-vm#946

Also update the tsconfig to target current versions of JavaScript in
build and in lib to use.
@locks
Copy link
Contributor

locks commented Dec 9, 2020

Closing as solved by #1053.

@locks locks closed this as completed Dec 9, 2020
@Herriau
Copy link

Herriau commented Dec 12, 2020

@locks Are we sure this is solved? I'm running into this exact same issue when attempting to use @glimmer/syntax v0.69.3 inside my add-on package (typescript v4.0.2).

Edit: I tried every minor versions of typescript between (and including) v3.5 and v4.1, and could not find one where imports from @glimmer/syntax doesn't cause my tsc build to fail. Adding skipLibCheck suppresses the build errors but this isn't ideal.

@dfreeman
Copy link
Contributor

@Herriau is it possible you have copies of multiple different versions of @glimmer/syntax in your dependency tree? I can confirm that I've got projects on several different versions of both TypeScript and @glimmer/syntax that are typechecking correctly these days.

@Herriau
Copy link

Herriau commented Dec 15, 2020

@dfreeman Indeed that must be the reason for this. I tried in a vanilla package (i.e. as opposed to my EmberCLI-generated add-on package) and I am not getting the same TS errors. I should be able to pin the version of @glimmer/syntax to one that includes the fix.

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

6 participants