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

Improve compile package detection in PNPM apps #840

Merged
merged 7 commits into from
Jun 23, 2023

Conversation

askoufis
Copy link
Contributor

This PR enables sku to detect compile packages inside the PNPM virtual store (node_modules/.pnpm). The current implementation of compile package detection only finds @seek scoped packages at the root of node_modules, so if you're not using PNPM with node-linker=hoisted, then sku can't find compile packages that are transitive dependencies (i.e. inside the PNPM virtual store).

Was also seeing some flakiness in the lint-format test snapshots where sometimes jest was reporting that the snapshots failed with diffs that looked identical except for the text colours. This lead me to try stripping ANSI escape codes in the snapshot serializer, and this seems to have worked AFAICT.

The storybook tests also decided to be flakey today, so I've increased the timeout on one of the queries we do and that seems to have helped.

@askoufis askoufis requested a review from a team as a code owner June 22, 2023 06:04
@changeset-bot
Copy link

changeset-bot bot commented Jun 22, 2023

🦋 Changeset detected

Latest commit: 78b90ab

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

@askoufis askoufis changed the title Pnpm compile packages Improve compile package detection in PNPM apps Jun 22, 2023
Copy link
Contributor

@mrm007 mrm007 left a comment

Choose a reason for hiding this comment

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

Nice work cracking that nut!

packages/sku/context/defaultCompilePackages.js Outdated Show resolved Hide resolved
@askoufis
Copy link
Contributor Author

Tested in an app and metropolis ✅

@askoufis askoufis merged commit e10e8d8 into master Jun 23, 2023
5 checks passed
@askoufis askoufis deleted the pnpm-compile-packages branch June 23, 2023 04:21
@seek-oss-ci seek-oss-ci mentioned this pull request Jun 23, 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

3 participants