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

Moving ember-cli-typescript to dependencies #358

Merged
merged 1 commit into from
Jan 12, 2022
Merged

Conversation

scalvert
Copy link
Collaborator

As indicated by the package, this should be a dependency, not a devDep.

@scalvert scalvert added the bug Something isn't working label Jan 12, 2022
@scalvert scalvert requested a review from rwjblue January 12, 2022 16:52
@scalvert scalvert merged commit 3d4d6ce into master Jan 12, 2022
@scalvert scalvert deleted the fix-e-c-t-dep branch January 12, 2022 18:42
@rwjblue
Copy link
Member

rwjblue commented Jan 12, 2022

Hmm, this doesn't seem quite right. We intentionally do not publish .ts files (so we shouldn't need to include this as a dep)...

@scalvert
Copy link
Collaborator Author

Ya I think the message in the console is not nuanced/is misleading:

WARNING: `ember-cli-typescript` should be included in your `dependencies`, not `devDependencies`

I was trusting that direction, but thinking more it does make sense what you're saying, and this message should be updated to apply in the following way:

  • for packages that publish .js files to consumers, devDependencies is fine
  • for packages that publish .ts files to consumers, dependencies is required.

@scalvert
Copy link
Collaborator Author

Happy to revert in light of this. It's not published yet.

@chriskrycho
Copy link

Just came across this and: yeah, we'll need to update that in light of v2 addons.

@scalvert
Copy link
Collaborator Author

scalvert commented Mar 1, 2022

This was reverted in #360

@chriskrycho
Copy link

Yep! That’s how I got here actually. 😅

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants