Skip to content

fix: remove implicit global references#932

Merged
ph-fritsche merged 3 commits intoglobals-mergefrom
globals-fix
Apr 17, 2022
Merged

fix: remove implicit global references#932
ph-fritsche merged 3 commits intoglobals-mergefrom
globals-fix

Conversation

@ph-fritsche
Copy link
Copy Markdown
Member

What:

Remove global references or make them explicit.

Why:

There might be problems with shadowed global variables in testing environments. See #892
The way e.g. jest hoists code we cannot be sure that a reference to e.g. window in a module is a reference to globalThis.window. We further cannot be sure that e.g. document.defaultView references the same object as window.
Although this is probably no issue in most cases we should be explicit about where we access objects through variables the user provided per setup() and where we access through global variables.

#929 , #931 enforce to be explicit about access to globals. Any reference that can't be found per text search for globalThis. results in a linting error.

How:

  • Remove global references from Clipboard util. We already pass down window to the create functions so we can use the scoped variable in the class.
    We still access the global window for our afterEach and afterAll hooks to reset/detach the Clipboard stub. This is probably not an issue as the environments providing a global after* hook are probably identical to environments merging window into the global scope (like jest+jsdom).
    If you happen to use an environment where you need to work around this, please leave a comment.
  • Make global references in setup() explicit. If the user provides a document with defaultView, these won't be used.
  • Make global references (parseInt, setTimeout) in utils explicit.

Checklist:

  • Tests
  • Ready to be merged

@codesandbox-ci
Copy link
Copy Markdown

codesandbox-ci Bot commented Apr 17, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit e7dc587:

Sandbox Source
userEvent-dom Configuration
userEvent-react Configuration

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 17, 2022

Codecov Report

❗ No coverage uploaded for pull request base (globals-merge@31a808b). Click here to learn what that means.
The diff coverage is n/a.

@@                Coverage Diff                @@
##             globals-merge      #932   +/-   ##
=================================================
  Coverage                 ?   100.00%           
=================================================
  Files                    ?        85           
  Lines                    ?      1784           
  Branches                 ?       641           
=================================================
  Hits                     ?      1784           
  Misses                   ?         0           
  Partials                 ?         0           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 31a808b...e7dc587. Read the comment docs.

@ph-fritsche ph-fritsche changed the base branch from main to globals-merge April 17, 2022 08:43
@ph-fritsche ph-fritsche merged commit 9913798 into globals-merge Apr 17, 2022
@ph-fritsche ph-fritsche deleted the globals-fix branch April 17, 2022 08:48
@github-actions
Copy link
Copy Markdown

🎉 This PR is included in version 14.1.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant