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

Use ember preview types #1272

Merged
merged 2 commits into from
Dec 16, 2022
Merged

Conversation

gitKrystan
Copy link
Collaborator

@gitKrystan gitKrystan commented Nov 30, 2022

Depends on #1271
Should unblock fixing emberjs/ember.js#20254

@@ -58,9 +58,11 @@ const Owner = EmberObject.extend(RegistryProxyMixin, ContainerProxyMixin, {
* @see {@link https://github.com/emberjs/ember.js/blob/v4.5.0-alpha.5/packages/%40ember/engine/instance.ts#L152-L167}
*/
unregister(fullName: string) {
// @ts-expect-error
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

These types aren't public and there is a bunch of EmberObject and as any in this file, so I figured leaving @ts-expect-error was OK...

Copy link
Contributor

Choose a reason for hiding this comment

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

That's the best move, yep. We can flag it up for figuring out a non-private-API way of tackling in the future!

@gitKrystan gitKrystan force-pushed the preview-types branch 2 times, most recently from b15da29 to 500e7b7 Compare December 14, 2022 19:55
"@types/ember__runloop": "~4.0.1",
"@types/ember__service": "~4.0.0",
"@types/ember__test": "~4.0.0",
"@types/jquery": "^3.5.14",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is needed because @types/ember-testing-helpers was removed.

@@ -9,5 +9,5 @@
"*": ["./types/*"]
}
},
"include": ["./addon-test-support/**/*.ts", "tests/**/*.ts"]
"include": ["addon-test-support/**/*.ts", "tests/**/*.ts", "types/**/*.ts"]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was necessary for the types to see types/dummy/index.d.ts

@@ -13,5 +13,6 @@
"types/*"
]
}
}
},
"include": ["api.ts", "../types/**/*"]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This was necessary for the types to see types/dummy/index.d.ts

Copy link
Contributor

@chriskrycho chriskrycho left a comment

Choose a reason for hiding this comment

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

Seems good! Thank you!

@@ -58,9 +58,11 @@ const Owner = EmberObject.extend(RegistryProxyMixin, ContainerProxyMixin, {
* @see {@link https://github.com/emberjs/ember.js/blob/v4.5.0-alpha.5/packages/%40ember/engine/instance.ts#L152-L167}
*/
unregister(fullName: string) {
// @ts-expect-error
Copy link
Contributor

Choose a reason for hiding this comment

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

That's the best move, yep. We can flag it up for figuring out a non-private-API way of tackling in the future!

@chriskrycho chriskrycho merged commit 5e5b11c into emberjs:master Dec 16, 2022
@gitKrystan gitKrystan deleted the preview-types branch December 16, 2022 18:41
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.

None yet

2 participants