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

refactor: import jest as global; unify import style of some modules #6898

Merged
merged 2 commits into from
Mar 11, 2022

Conversation

Josh-Cena
Copy link
Collaborator

Motivation

I spent a while playing with running Jest in ESM. I was very close to success. Now, I'm a bit more confident about how to roll this out gradually—as usual, the migration (even when it's only for development) will be split into several granular PRs so people can follow the progress and catch any bugs in the process.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Tests still pass

Related PRs

#6520

@Josh-Cena Josh-Cena added the pr: maintenance This PR does not produce any behavior differences to end users when upgrading. label Mar 11, 2022
@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Mar 11, 2022
@netlify
Copy link

netlify bot commented Mar 11, 2022

✔️ [V2]

🔨 Explore the source changes: 03853de

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/622b25e98b6fd6000960a950

😎 Browse the preview: https://deploy-preview-6898--docusaurus-2.netlify.app

@github-actions
Copy link

github-actions bot commented Mar 11, 2022

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 59
🟢 Accessibility 100
🟢 Best practices 92
🟢 SEO 100
🟢 PWA 90

Lighthouse ran on https://deploy-preview-6898--docusaurus-2.netlify.app/

@github-actions
Copy link

github-actions bot commented Mar 11, 2022

Size Change: -24 B (0%)

Total Size: 792 kB

ℹ️ View Unchanged
Filename Size Change
website/.docusaurus/globalData.json 49.9 kB 0 B
website/build/assets/css/styles.********.css 105 kB 0 B
website/build/assets/js/main.********.js 598 kB -24 B (0%)
website/build/index.html 38.7 kB 0 B

compressed-size-action

@Josh-Cena Josh-Cena changed the title refactor: import jest as global refactor: import jest as global; unify import style of some modules Mar 11, 2022
@@ -96,7 +95,9 @@ const useAnnouncementBarContextValue = (): AnnouncementBarAPI => {
);
};

const AnnouncementBarContext = createContext<AnnouncementBarAPI | null>(null);
const AnnouncementBarContext = React.createContext<AnnouncementBarAPI | null>(
Copy link
Collaborator

Choose a reason for hiding this comment

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

not against this change, just wondering why it's necessary?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Oh, purely stylistic. We have some React.createContext and some createContext. I personally prefer to have the React. prefix except for hooks so we understand what we are talking about

{
files: ['*.test.ts', '*.test.tsx'],
rules: {
'import/no-extraneous-dependencies': OFF,
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe this should be disabled with more granularity (if possible)? 🤷‍♂️

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Test files are internal anyways. If they can run, there's nothing to worry about.

@Josh-Cena Josh-Cena merged commit c9ee6e4 into main Mar 11, 2022
@Josh-Cena Josh-Cena deleted the jc/global-jest branch March 11, 2022 11:04
@Josh-Cena Josh-Cena mentioned this pull request Mar 11, 2022
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: maintenance This PR does not produce any behavior differences to end users when upgrading.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants