fix: fix more types #2893
fix: fix more types #2893
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
| const l = tokens.length; | ||
| for (i = 0; i < l; i++) { | ||
| token = tokens[i]; | ||
| for (let i = 0; i < tokens.length; i++) { |
There was a problem hiding this comment.
Is there any perf difference between storing tokens.length in a variable vs checking in here each time? I suspect we won't see much of a different if the length is small but it might add up for large arrays.
There was a problem hiding this comment.
There is no performance benefit from storing the variable first. https://stackoverflow.com/a/17995000/806777
Co-authored-by: Steven <[email protected]>
Co-authored-by: Steven <[email protected]>
| startInline: Array<(this: TokenizerThis, src: string) => number | void> | ||
| }) | ||
| | undefined | null; | ||
| walkTokens?: null | ((token: Token) => void | (unknown | Promise<void>)[]); |
There was a problem hiding this comment.
This breaks downstream compilation because (unknown | Promise<void>)[] gets collapsed to unknown[] jupyterlab/jupyterlab#15020 see https://stackoverflow.com/questions/61685819/typescript-does-not-recognize-a-union-of-type-unknown-and-type-promiseunknown
Should we adapt the type donwstream, or do you consider it a bug?
There was a problem hiding this comment.
@UziTech Perhaps this should be (void | Promise<void>)[] instead?
There was a problem hiding this comment.
Ya I think (void | Promise<void>)[] should work
There was a problem hiding this comment.
The only time we care about the return value is if async is true. Than we await the return value. I think the problem was that we can't await void in typescript but it is perfectly fine in JavaScript.
There was a problem hiding this comment.
The only time we care about the return value is if async is true.
Maybe we need different types when async: true if we want it to be more precise
There was a problem hiding this comment.
Should be fixed by #2955
Description
Make types more strict
Contributor
Committer
In most cases, this should be a different person than the contributor.