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

EmberData's native types don't compile with glint #9446

Closed
pieter-v opened this issue May 17, 2024 · 9 comments
Closed

EmberData's native types don't compile with glint #9446

pieter-v opened this issue May 17, 2024 · 9 comments

Comments

@pieter-v
Copy link
Contributor

Reproduction

You can simply reproduce these errors by creating an empty embroider project with glint and template-imports enabled:

  1. pnpm dlx ember new test-types -pnpm --embroider --typescript
  2. add import 'ember-source/types'; to types/<your-app>/index.d.ts
  3. install @ember-data-types@5.4.0-alpha.64 as described in the guides
  4. install glint with template-imports

This example repo can be found here: https://github.com/pieter-v/test-types

Running ember build gives no errors.
Running glint (or tsc) gives a large list of errors.

Description

The role for running glint in such a project is crucial as the error and type checking is done by glint and not by ember build.

Versions

Legend: production dependency, optional only, dev only

test-types@0.0.0 /Users/amcgs/tmp/test-types

devDependencies:
@ember-data-types/adapter 5.4.0-alpha.64
@ember-data-types/graph 5.4.0-alpha.64
@ember-data-types/json-api 5.4.0-alpha.64
@ember-data-types/legacy-compat 5.4.0-alpha.64
@ember-data-types/model 5.4.0-alpha.64
@ember-data-types/request 5.4.0-alpha.64
@ember-data-types/request-utils 5.4.0-alpha.64
@ember-data-types/serializer 5.4.0-alpha.64
@ember-data-types/store 5.4.0-alpha.64
@ember-data-types/tracking 5.4.0-alpha.64
ember-data 5.3.3
ember-data-types 5.4.0-alpha.64
@runspired
Copy link
Contributor

haven't had time to poke through the project you posted; however, this seems almost certainly to be a config issue. We use glint within EmberData itself already, and use it at work with EmberData's native types.

@runspired
Copy link
Contributor

ok I pulled this down and confirmed a suspicion: the problem here is that neither the project's tsconfig nor the @tsconfig/ember base specifies skipLibCheck: true, which it ought to.

There's no way for libraries to avoid shipping errors in types to consumers (literally), so skipLibCheck should always be used.

cc @NullVoxPopuli / @wagenet / @gitKrystan

@NullVoxPopuli
Copy link
Sponsor Contributor

NullVoxPopuli commented May 19, 2024

skipLibCheck: true, which it ought to.

idk, it falls back to any when library code goes wrong, which hides problems 🙈

I would decline PRs to @tsconfig/ember for this change, as I think it's not something library authors should ever enable, and something appdevs could enable, knowing the risks.

@runspired
Copy link
Contributor

@NullVoxPopuli then the default for Ember apps will be that typechecking fails 🤷🏻‍♂️

its literally impossible to not ship broken types. tsc strips @ts-expect-error and @ts-ignore-error from dts files, and it creates dts files for untyped code that are horribly broken when you generate the types for what code actually is typed.

@runspired
Copy link
Contributor

also its just literally never a good idea to have skipLibCheck: false, tbh its shocking typescript hasn't removed the feature. Setting it to false tanks your performance, doesn't work with monorepos, and leaves you highly susceptible to semver drift breaking builds while offering no improved type safety or checks.

@NullVoxPopuli
Copy link
Sponsor Contributor

NullVoxPopuli commented May 19, 2024

why is a library shipping with expect/ignore error tho? or trying to? I feel like there is some context I don't understand where there are broken types somewhere in the dep graph, and a library is trying to paper over the problem with those directives?

My main concern is silent fallback to any from libraries and libraries' internal types (some deep library types are wrong). as long as that doesn't happen, I'm happy with whatever the config ends up being.

@runspired
Copy link
Contributor

My main concern is silent fallback to any from libraries and libraries' internal types (some deep library types are wrong). as long as that doesn't happen, I'm happy with whatever the config ends up being.

this is both something no app can ever fix and will happen anyway regardless, so its not worth it even yelling at you by default. tsc by-default produces any or types that resolve to any for tons of types. Especially if you use jsdoc, it prefers jsdoc to actual type information and creates non-existent symbols for non-existent types.

@runspired
Copy link
Contributor

why is a library shipping with expect/ignore error tho?

also because good libraries care about shipping correct-for-consumer types, which means they have to use ts-expect-error in order to produce the correct type.

@NullVoxPopuli
Copy link
Sponsor Contributor

NullVoxPopuli commented May 19, 2024

you probably want to cast to unknown as TheTypeYouReallyWant as ts-expect-error has never worked for libraries -- just not an option 😅

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

No branches or pull requests

3 participants