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

chore: create monorepo e2e spec #1079

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

rmarscher
Copy link
Contributor

This needs more work to figure out how to install dependencies in the test monorepo and link in the waku package in a way that closely matches the way it works with published npm packages.

Copy link

vercel bot commented Dec 18, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Skipped Deployment
Name Status Preview Updated (UTC)
waku ⬜️ Ignored (Inspect) Visit Preview Jan 5, 2025 10:25pm

Copy link

codesandbox-ci bot commented Dec 18, 2024

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.

@dai-shi
Copy link
Owner

dai-shi commented Dec 18, 2024

@himself65 Can you help this?

@rmarscher
Copy link
Contributor Author

Thank you for the help. Basically, I am trying to do these steps:

cd /tmp
cp -r ~/waku/e2e/fixtures/monorepo .
cd monorepo
npm i
cd packages
npm i --save waku@latest
npm run dev

^^ and then verify it loads the home page without errors

It should install waku at /tmp/monorepo/node_modules which has then causes some different behavior with vite paths in the past because it is outside the /tmp/monorepo/packages/waku-project root dir.

When I run the above commands and it installs 0.21.9, it is working.

If I instead npm i --save [email protected], I get this error:

Cannot process SSR Error [ERR_PACKAGE_PATH_NOT_EXPORTED]: Package subpath './router/client.js?v=1ef4d55f' is not defined by "exports" in /tmp/monorepo/node_modules/waku/package.json imported from /tmp/monorepo/node_modules/waku/dist/lib/middleware/dev-server-impl.js

@dai-shi dai-shi mentioned this pull request Dec 22, 2024
@dai-shi dai-shi linked an issue Dec 22, 2024 that may be closed by this pull request
@dai-shi dai-shi mentioned this pull request Dec 22, 2024
Copy link
Contributor

@himself65 himself65 left a comment

Choose a reason for hiding this comment

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

I think this structure is not working as expected with pnpm monorepo?
pnpm will resolve all deps in the root node_modules, which I think the current e2e cannot really test the prod case?

@dai-shi
Copy link
Owner

dai-shi commented Dec 28, 2024

@himself65 Can we do something like the standalone setup? It uses npm though.

@himself65
Copy link
Contributor

@himself65 Can we do something like the standalone setup? It uses npm though.

Im trying to do that

@himself65
Copy link
Contributor

I was trying to do that in #920

@rmarscher rmarscher force-pushed the chore/monorepo-spec branch from 627aed0 to 7412111 Compare January 5, 2025 07:58
@rmarscher
Copy link
Contributor Author

rmarscher commented Jan 5, 2025

I got the e2e test to work by passing a package path to set the cwd for the commands. https://github.com/dai-shi/waku/pull/1079/files#diff-810f0b71160f0ddae45c40c1b3b4534a9a48171823eebf14d7d67ba557f6f4ad

I like how #1101 has the ability to test a few different package managers. We can close this PR and track there instead if it is better.

I see even though e2e tests are working, something is broken in the regular test suite checking imports. I copied the standard create-waku template to the monorepo example. I think some of the monorepo errors we have seen in the past were related to fs router.

@rmarscher rmarscher marked this pull request as ready for review January 5, 2025 20:42
@rmarscher
Copy link
Contributor Author

I fixed the tests with // eslint-disable-line import/no-unresolved. 😕 🤷

@dai-shi
Copy link
Owner

dai-shi commented Jan 5, 2025

I like how #1101 has the ability to test a few different package managers. We can close this PR and track there instead if it is better.

@himself65 seems busy, so making this pr incorporate #1101 sounds good.

@rmarscher
Copy link
Contributor Author

Great. I did that. All seems to be passing.

@rmarscher rmarscher requested a review from himself65 January 6, 2025 00:19
execSync(
`node ${join(standaloneDir, './node_modules/waku/dist/cli.js')} build`,
{ cwd: standaloneDir },
{ cwd: join(standaloneDir, packageDir) },
Copy link
Owner

Choose a reason for hiding this comment

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

Make it consistent with L186. Either use join for both, or use string concatenation for both. L209 too.

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.

Issue with monorepo?
3 participants