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

topic/289-typescript #293

Merged
merged 8 commits into from
Sep 29, 2021
Merged

topic/289-typescript #293

merged 8 commits into from
Sep 29, 2021

Conversation

lfrost
Copy link
Contributor

@lfrost lfrost commented Sep 27, 2021

This includes the basic wiring for TypeScript and the conversion of one JavaScript file to TypeScript (as a first step for issue 289).

    yarn upgrade ember-cli@~3.27.0

Will need to use `ember install`, and this is broken in 3.28.0.
eslint-plugin-ember@10.5.5, prettier@2.4.1, qunit@2.17.2,
release-it@14.11.6, broccoli-funnel@3.0.8
    ember install ember-cli-typescript

Manually moved ember-cli-typescript from devDependencies to
dependencies.  See https://docs.ember-cli-typescript.com/ts/with-addons#key-differences-from-apps
    yarn add --dev @typescript-eslint/eslint-plugin @typescript-eslint/parser typedoc
@lfrost lfrost mentioned this pull request Sep 27, 2021
Copy link
Contributor

@jherdman jherdman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for doing this. Just a few comments above.

.eslintrc.js Outdated Show resolved Hide resolved
"babel-eslint": "^10.1.0",
"broccoli-asset-rev": "^3.0.0",
"ember-cli": "~3.28.0",
"ember-cli": "~3.27.0",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems like a mistake.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, it is a workaround for a bug in ember-cli 3.28.0. Now that ember-cli-typescript is installed, we could upgrade back to 3.28.0, but I would wait for 3.28.1 to be released.

Copy link
Contributor

@jherdman jherdman Sep 28, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just this change left.

Edit: just saw your comment above.

Copy link
Contributor

@jherdman jherdman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One last note.

@@ -2,6 +2,7 @@

# compiled output
/dist/
/doc/
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this change be copied to .npmignore too?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. I did not notice the project had a .npmignore. I have pushed this change.

@jherdman jherdman merged commit 548602b into adopted-ember-addons:master Sep 29, 2021
@jherdman
Copy link
Contributor

Many thanks for your contribution! 😍

@jherdman
Copy link
Contributor

Resolves #289

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.

2 participants