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

[ENHANCEMENT] Use the official types in the blueprints #10506

Merged

Conversation

Windvis
Copy link
Contributor

@Windvis Windvis commented Sep 27, 2024

This updates the TS blueprints to use the official types instead of the outdated DT types.

Supersedes #10461 and #10270

@Windvis Windvis changed the title Use the official types in the blueprints [ENHANCEMENT] Use the official types in the blueprints Sep 27, 2024
@Windvis
Copy link
Contributor Author

Windvis commented Sep 27, 2024

I also found this open PR which is somewhat related: #10270

Do we also want to add some type import comments for the js users?

@Windvis

This comment was marked as outdated.

This matches the sorting of npm, so nothing is moved around after installing new packages.
Copy link
Contributor

@NullVoxPopuli NullVoxPopuli left a comment

Choose a reason for hiding this comment

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

Yaaaas

@kategengler kategengler merged commit 3d3985c into ember-cli:master Sep 30, 2024
41 checks passed
@kategengler
Copy link
Member

Thank you!

@Windvis Windvis deleted the use-official-types-in-blueprints branch September 30, 2024 07:08
@Windvis
Copy link
Contributor Author

Windvis commented Sep 30, 2024

FYI, I tested a similar setup (based on v5.3.8) in one of our projects but it complained about the paths in the tsconfig file. We need to prefix the EmberData packages with .node_modules/* for it to work, which is also what the warpdrive retrofit cli command does.

    "types": [
      "./node_modules/@ember-data-types/adapter/unstable-preview-types",
      "./node_modules/@ember-data-types/graph/unstable-preview-types",
      "./node_modules/@ember-data-types/json-api/unstable-preview-types",
      "./node_modules/@ember-data-types/legacy-compat/unstable-preview-types",
      "./node_modules/@ember-data-types/model/unstable-preview-types",
      "./node_modules/@ember-data-types/request-utils/unstable-preview-types",
      "./node_modules/@ember-data-types/request/unstable-preview-types",
      "./node_modules/@ember-data-types/serializer/unstable-preview-types",
      "./node_modules/@ember-data-types/store/unstable-preview-types",
      "./node_modules/@ember-data-types/tracking/unstable-preview-types",
      "./node_modules/@warp-drive-types/core-types/unstable-preview-types",
      "./node_modules/ember-data-types/unstable-preview-types",
      "ember-source/types"
    ]

I need to double check if v5.4.0.beta-11 has the same issue and I simply missed it here.

What is strange is that the ember-source/types path doesn't need the prefix. I'm not sure what's different between the two setups.

In any case, this potentially needs a follow up PR if the current setup isn't working out of the box.

Edit: it seems the issue is that the ember-data packages have an "exports" config but they don't include rules for type imports. This is by design, since the types are still unstable, but it does seem to influence the way the types array in the tsconfig file works.

Once we add the ./node_modules/ prefix, things start working again. Alternatively we could switch back to the types packages, but the ./node_modules prefix was suggested by @runspired in the Discord.

"@ember-data/request/unstable-preview-types",
"@ember-data/request-utils/unstable-preview-types",
"@ember-data/model/unstable-preview-types",
"@ember-data/serializer//unstable-preview-types",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I missed the //..

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.

5 participants