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
chore: migrate jest-environment-jsdom to TypeScript #8003
chore: migrate jest-environment-jsdom to TypeScript #8003
Changes from 25 commits
c857e4c
0cfe10b
57a3567
9b279a6
2c3afe4
ae3c45c
bd4e070
71048f6
cc4eacb
b3effe8
9cf040b
a40ea8d
b215884
0a9e4c4
4e8e638
3c4486f
d50560b
3968bc4
6105073
5ffc083
f45cc88
1f9081a
f1ed385
e162e48
eebb6f8
a743367
3beb007
5fb6bfd
58fc28b
c7c84f7
06539b1
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
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.
We know what the type here will be, no need to assert it.
(it'll have the
EVAL_RESULT_VARIABLE
object thing due to https://github.com/facebook/jest/blob/438a178c8addf2806bebf44726d080921ad9984f/packages/jest-transform/src/ScriptTransformer.ts#L559-L565)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.
We do need to assert it since
runVMScript()
returnsvoid
, which is not true, andrunScript()
is also expected to receive{[ScriptTransformer.EVAL_RESULT_VARIABLE]: ModuleWrapper} | null
(via the JestEnvironment interface). I have a new solution for this, committing in a sec.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 contrasts with #8003 (comment) - we can't both set the return type to the
EVAL_RESULT_VARIABLE
thing, AND the return value of the provided script... Sorry, I'm a bit confused here.If we want the return type of
runScript
to be the the same as the return type of the provided script, we could use a generic to let the user ofjest-environment-jsdom
set the return type? Just an idea. But I feel there is something I am missing here... :)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 return value of the provided script is, 100%, the
EVAL_RESULT_VARIABLE
thing (unless the environment has been torn down, in which cse it will benull
). We construct the script ourselves. The function assigned toEVAL_RESULT_VARIABLE
, we do not know what will return when invoked (that's theunknown
part), but we don't care about that resultI don't think that's needed - the script we get is constructed from a string source we've read (and potentially transformed) from disk. So we cannot statically know what it will return