-
-
Notifications
You must be signed in to change notification settings - Fork 155
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
Spike on TS types #992
Spike on TS types #992
Conversation
"@glimmer/interfaces": "^0.84.2", | ||
"@glimmer/reference": "^0.84.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IDK why I had to install these but I had a lot of typescript errors in node_modules without them.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, that seems very bad. I will check out the PR and see if I can understand why.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For what it's worth, after removing the yalc
bit, I don't see this. I haven't yet tried it with a yalc
'd @ember/test-helpers
, though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll try again now that everything is further along.
@@ -0,0 +1,3 @@ | |||
// FIXME: Not sure this actually does anything |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This definitely should do a thing, but it depends on how paths
is set I think. This may be basically-invisible as it stands; this is a problem with our blueprint and config that has existed for a very long time.
@@ -0,0 +1,7 @@ | |||
// Types for compiled templates |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can probably remove this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep—in fact we definitely want to! They don't get us anything useful in most apps/addons, but they are completely useless in this one in particular I think.
@@ -59,7 +68,7 @@ | |||
"ember-disable-prototype-extensions": "^1.1.3", | |||
"ember-load-initializers": "^2.1.2", | |||
"ember-resolver": "^8.0.3", | |||
"ember-source": "~4.9.1", | |||
"ember-source": "^4.10.0-beta.2", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What needs -beta.2
? I assume the Owner
types?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Resolver
preview types from beta.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Makes sense. I just landed a PR this morning, and will do the back port for it on Monday, which will make it available (along with other Owner
types needed) on 4.8 LTS.
Superseded by #994 |
See #957
This is what would be published:
I have lots of questions below.
Requirements before merge
emberjs/ember-test-helpers#1269
emberjs/ember-test-helpers#1270
emberjs/ember-test-helpers#1271
emberjs/ember-test-helpers#1272
emberjs/ember-test-helpers#1277
and [email protected]