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

TypeScript conversion so we have accurately generated types for consumers #275

Merged
merged 14 commits into from
Jan 8, 2024

Conversation

NullVoxPopuli
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli commented Dec 27, 2023

These 3 PRs should be merged first:

As ember has first-class TypeScript support out of the box (almost), we need to to have all core addons also support that.

Like always, consumers don't need to use TypeScript, but our library code using TypeScript will provide better intellisense for both JavaScript and TypeScript users.

Incidentally, achieving more alignment with the TS-v2-addon blueprint, we gain support for Glint-using projects.

This work continues the work of #246

Demo in the docs app:
image

Changes:

  • convert to TS
  • npx ember-apply typescript to the docs app
  • set up Glint in the docs app
  • test with Glint in the docs app
  • upgrade the test-app to latest ember-source -- (fastboot + embroider) is not bug free with ember-source 3.28, but is with latest ember-source. Going to inquire with the embroider folks if this combo should be supported. It could also be that the 3.28 folks need to use an earlier version of embroider -- idk how complex / thorough we want the test matrix to be

NullVoxPopuli added a commit that referenced this pull request Dec 27, 2023
@NullVoxPopuli NullVoxPopuli changed the base branch from master to pull-forward-lint-changes-from-ts-branch December 27, 2023 17:31
@NullVoxPopuli NullVoxPopuli marked this pull request as ready for review December 27, 2023 23:20
@NullVoxPopuli
Copy link
Contributor Author

Current issue with the embroider tests.
image

what in tarnation is going on. broccoli doesn't go in the browser.

NullVoxPopuli added a commit that referenced this pull request Dec 28, 2023
NullVoxPopuli added a commit that referenced this pull request Dec 28, 2023
…-ts-branch

Pull forward the lint changes from #275
Base automatically changed from pull-forward-lint-changes-from-ts-branch to master December 28, 2023 15:04
@@ -6,6 +6,6 @@
{{@title}}
</header>
<div>
{{yield this}}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is a template-only component, and thus has no this

Copy link

@wagenet wagenet left a comment

Choose a reason for hiding this comment

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

I didn't verify every single line, but the overall direction looks correct to me.

@NullVoxPopuli NullVoxPopuli merged commit 7effdc8 into master Jan 8, 2024
22 checks passed
@NullVoxPopuli NullVoxPopuli deleted the typescript-take-two branch January 8, 2024 16:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants