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

feat: install @types/ember packages separately via blueprint #1383

Merged
merged 2 commits into from
Nov 15, 2020

Conversation

mydea
Copy link
Contributor

@mydea mydea commented Nov 13, 2020

This PR updates the install blueprint to install the @types/ember__XXX packages separately, instead of installing only @types/ember.

The problem is that TS intentionally does not suggest imports from transitive dependencies, since it's not safe to depend on them;

Some notes:

  • I did not install @types/ember itself - I have been running our app like this for a while, and I think you'd only need it if you use the Ember namespace (e.g. import Ember from 'ember'; Ember.get), which is not encouraged anymore anyhow and can/should be mostly avoided.
  • For @types/ember-data I did not find any direct dependencies - not sure how exactly that works there with e.g. @types/ember-data__model, I guess you'd have to manually install those even now?

Closes #1341

@chriskrycho chriskrycho requested review from dfreeman and jamescdavis and removed request for dfreeman November 13, 2020 17:44
@chriskrycho
Copy link
Member

@mydea thank you so much for opening this!

@dfreeman I believe you most recently poked at this—my understanding is we probably do still want to install @types/ember directly as well, even though we’re not normally importing from the Ember namespace directly anymore, if for no other reason than that dependency management (esp. w/any transitives) is much easier if it’s direct. Does that sound right to you?

@dfreeman
Copy link
Member

I think the extent of my poking (unless I'm forgetting another conversation) was what @mydea included in the description—something along the lines of "oh look, auto-import works if the corresponding @types/ember__ package is a direct dependency 🤔"

I think the only reason to still include @types/ember is if you need to import from the 'ember' module, which is mostly only necessary if you're using very legacy APIs.

The one exception is the array prototype extensions, which it looks like we still enable types for by default. Since the source of truth for those is in @types/ember, we likely either need to continue installing that by default or comment out that line in the blueprint and include a note that you need @types/ember for those to work.

@chriskrycho
Copy link
Member

Ah, okay, that makes sense. I think I'm fine with changing the default here, though we'll want to note it and make sure it gets hit in the docs (if/as/when we actually finish updating them 😭).

@chriskrycho
Copy link
Member

Landing this—we can iterate on the @types/ember question later!

@chriskrycho chriskrycho merged commit 8f5758a into typed-ember:master Nov 15, 2020
@mydea mydea deleted the fn/ember-packages branch November 17, 2020 07:07
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

Successfully merging this pull request may close these issues.

@types/ember is not used for auto completion
3 participants