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

Fix root resolution within .css.ts files during tests #741

Merged
merged 1 commit into from
Feb 27, 2023

Conversation

askoufis
Copy link
Contributor

@askoufis askoufis commented Feb 24, 2023

Replacing the VE babel plugin with the VE jest transform resulted in .css.ts files no longer going through our babel transformation stack during jest tests. This ended up breaking root resolution (importing from src/*) within .css.ts files during tests.

Luckily, jest has a config value just for this: modulePaths. Setting this to the result of cwd when rootResolution is true (like we do in the babel transform stack) fixes the issue.

@askoufis askoufis requested a review from a team as a code owner February 24, 2023 03:41
@changeset-bot
Copy link

changeset-bot bot commented Feb 24, 2023

🦋 Changeset detected

Latest commit: 3916036

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
sku Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -10,6 +11,7 @@ module.exports = {
testEnvironment: 'jsdom',
setupFilesAfterEnv: paths.setupTests,
prettierPath: require.resolve('prettier'),
modulePaths: rootResolution ? [cwd()] : undefined,
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this work?

Suggested change
modulePaths: rootResolution ? [cwd()] : undefined,
modulePaths: rootResolution ? ['<rootDir>'] : undefined,

Copy link
Contributor Author

@askoufis askoufis Feb 24, 2023

Choose a reason for hiding this comment

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

It probably does, but I wanted to keep it consistent with the babel transform root, which we get from the result of cwd.

root: rootResolution ? [cwd()] : undefined,

I think if you're running sku test via your package manger, this will end up being your project root anyway.

@askoufis askoufis requested a review from mrm007 February 24, 2023 04:51
@askoufis askoufis merged commit ccba987 into master Feb 27, 2023
@askoufis askoufis deleted the root-resolution-ve-jest branch February 27, 2023 04:24
@seek-oss-ci seek-oss-ci mentioned this pull request Feb 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants