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

build(Tabs): updates reach/tabs #2879

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

timjenkins
Copy link
Contributor

@timjenkins timjenkins commented May 29, 2024

Overview

Updates @reach/tabs to version 18, Adds webpack rule for storybook build to transpile @reach modules

PR Checklist

  • Related to designs:
  • Related to JIRA ticket: GM-561
  • I have run this code to verify it works
  • This PR includes unit tests for the code change
  • This PR includes testing instructions tests for the code change
  • The alpha package of this PR is passing end-to-end tests in all relevant Codecademy repositories

Testing Instructions

  • Open storybook/tabs
  • Test out all the tabs
  • Make sure they work and there are no console errors

Don't make me tap the sign.

PR Links and Envs

Repository PR Link PR Env
Monolith Monolith PR [Monolith Env]
Portal Portal Link [Portal Env]

@timjenkins timjenkins marked this pull request as ready for review May 29, 2024 15:31
@timjenkins timjenkins requested a review from a team as a code owner May 29, 2024 15:31
@timjenkins timjenkins requested review from jakemhiller and BandanaKM and removed request for jakemhiller May 29, 2024 15:31
@timjenkins timjenkins requested review from LinKCoding and dreamwasp and removed request for BandanaKM May 29, 2024 15:32
@timjenkins timjenkins added dependencies Pull requests that update a dependency file Auto Update Automatically keep this PR up to date labels May 29, 2024
Copy link
Contributor

@dreamwasp dreamwasp left a comment

Choose a reason for hiding this comment

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

code looks good! CI is failing in a couple places in monorepo + monolith -- just holding off approving until that all looks good

Comment on lines +56 to +78
const transpileModules = ['@reach'];

// Find existing rule that excludes node_modules
const nodeModulesRule = config.module.rules?.find((rule: any) =>
rule.exclude?.toString().includes('node_modules')
);
if (nodeModulesRule) {
// Tell existing rule to not exclude modules that need transpiling
const newExclude = new RegExp(
`node_modules/(?!(${transpileModules.join('|')})/).*`
);

if (Array.isArray(nodeModulesRule.exclude)) {
nodeModulesRule.exclude = [
newExclude,
...nodeModulesRule.exclude?.filter(
(exclude: any) => !exclude.toString().includes('node_modules')
),
];
} else {
nodeModulesRule.exclude = newExclude;
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

ooo, this is cool!

@codecademydev
Copy link
Collaborator

📬Published Alpha Packages:

@codecademy/[email protected]
@codecademy/[email protected]
@codecademy/[email protected]

@codecademydev
Copy link
Collaborator

🚀 Styleguide deploy preview ready!

https://66674c27b7f3d932ba5e680f--gamut-preview.netlify.app

Deploy Logs

@@ -10,7 +10,7 @@
"@codecademy/gamut-styles": "16.3.0",
"@codecademy/variance": "0.21.3",
"@reach/auto-id": "^0.16.0",
Copy link
Member

Choose a reason for hiding this comment

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

i think we should to bump this too if possible, to avoid having multiple versions in the lockfile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Auto Update Automatically keep this PR up to date dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants