Skip to content
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

make jsdom accessible to extending environments #12232

Merged
merged 2 commits into from Feb 10, 2022

Conversation

robin-drexler
Copy link
Contributor

@robin-drexler robin-drexler commented Jan 13, 2022

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 dom has been made private.

This causes extending environments that depend on this.dom being available like jest-environment-jsdom-global to break if they use TypeScript.

The error you get if you try to access this.dom in an environment:

 Property 'dom' is private and only accessible within class 'JSDOMEnvironment'.
export class JSDOMEnvironmentGlobal extends JSDOMEnvironment {
  constructor(config, options) {
    super(config, options);

   // this line triggers the error
    this.global.jsdom = this.dom;
  }
}

I prepared a small reproduction repository.
It uses a custom TS environment that tries to access this.dom and fails.

To see for yourself:

  • clone the repo
  • run npm install
  • run npx jest

jest-environment-jsdom-global itself is actually unaffected because it uses JS and the transpiled CommonJS version of jest-environment-jsdom doesn't enforce this.dom to be private. If jest changes its bundling process in the future, this could also fail for projects using JS.

I think it'd be great if jsdom continued to be accessible for custom environments since that enables use cases that would otherwise not be possible.

@robin-drexler
Copy link
Contributor Author

I guess that would introduce whatever the previous PR fixed. I'll create an issue instead first.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

@SimenB SimenB left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks!

@robin-drexler
Copy link
Contributor Author

@SimenB thanks for the guidance and review! 🙌

@SimenB SimenB merged commit 0d0844a into jestjs:main Feb 10, 2022
@github-actions
Copy link

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.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug]: make jsdom accessible to extending environments again
3 participants