make jsdom accessible to extending environments - #12232
Merged
Merged
Conversation
Contributor
Author
|
I guess that would introduce whatever the previous PR fixed. I'll create an issue instead first. |
robin-drexler
force-pushed
the
allow-dom-access
branch
from
February 10, 2022 13:08
334b1a0 to
48fb7f7
Compare
robin-drexler
marked this pull request as ready for review
February 10, 2022 13:08
SimenB
requested changes
Feb 10, 2022
SimenB
left a comment
Member
There was a problem hiding this comment.
thanks!
you also need to move https://github.com/facebook/jest/blob/df0c496fa2b39293c26967f66bada30c3dcfd3c4/packages/jest-environment-jsdom/package.json#L30 to dependencies and add a changelog entry 🙂
robin-drexler
force-pushed
the
allow-dom-access
branch
from
February 10, 2022 14:57
48fb7f7 to
34a39d9
Compare
robin-drexler
force-pushed
the
allow-dom-access
branch
from
February 10, 2022 14:58
34a39d9 to
6b19d6d
Compare
Contributor
Author
|
@SimenB thanks for the guidance and review! 🙌 |
robin-drexler
force-pushed
the
allow-dom-access
branch
from
February 10, 2022 15:25
9f448d2 to
1135197
Compare
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes: #12233
I initially wanted to create an issue, but thought it may be better if I opened a PR. Please let me know if you'd rather have me open an issue.
In f9814d2
domhas been made private.This causes extending environments that depend on
this.dombeing available like jest-environment-jsdom-global to break if they use TypeScript.The error you get if you try to access
this.domin an environment:I prepared a small reproduction repository.
It uses a custom TS environment that tries to access
this.domand fails.To see for yourself:
npm installnpx jestjest-environment-jsdom-globalitself is actually unaffected because it uses JS and the transpiled CommonJS version ofjest-environment-jsdomdoesn't enforcethis.domto be private. Ifjestchanges its bundling process in the future, this could also fail for projects using JS.I think it'd be great if
jsdomcontinued to be accessible for custom environments since that enables use cases that would otherwise not be possible.