Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
update to TypeScript v3.8-rc #57774
update to TypeScript v3.8-rc #57774
Changes from 9 commits
3018d18
4bab0a5
b9016ff
34b069d
c62c8df
6c8c249
f282992
647d2b7
c561ee4
82fcfdc
fa4334f
1ab9894
1c25e13
3e1e353
5fe5d5c
9d8c170
b3489d9
33d3f93
822b5bd
9d51788
f13a261
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
the file still contains some type errors. @timroes @sulemanof could someone take a look and fix the errors, please?
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.
@elastic/apm-ui I need your help here to fix
APMRequestHandlerContext
which type is inferred asany
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.
@dgieselaar fyi
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.
Looking into it!
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.
@restrry I can't explain this - there's no reason I can see why this fails, and others don't. It feels like a TS bug, but I cannot pinpoint it.
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.
btw - in my case, I don't see
context
as any, but the type forcontext.params
is incorrect.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.
@sqren is
context.params
type correct in your case?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.
Hmm. It seems like it's not. On master I get 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.
I think it might be related to the fact that
Params
interface contains optional props only.When I remove an empty object default from
kibana/x-pack/plugins/apm/server/routes/create_route.ts
Line 12 in 96a0aa0
TS starts inferring context type, but not params
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'm not here until Monday, and my only guess right now is that this is a TS bug (either in resolving or in showing an appropriate error). Can someone flag this with the TS team? If there's not much of a rush I can look into it Monday.
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.
@dgieselaar take your time, we are blocked by external dependencies as well