-
-
Notifications
You must be signed in to change notification settings - Fork 355
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
Some changes to make vite-ecosystem-ci pass #371
Conversation
I'm seeing that main is now failing, probably a minor thing I'll fix tomorrow. I'll reply to the rest tmrw as well. In the meantime: thanks for the PR! |
Alright, both the branch
I think so too: github:apollographql/apollo-client - PR #9933 - file changes.
I can confirm that In care you are curious:
Hopefully, Apollo fixes these issues soon. I don't think that using
That's probably because vite-plugin-ssr automatically runs (So that the user can simply run This means that the first I'm surprised though because I believe that vite-ecocystem-ci uses
Neat. I re-enabled Preact's test suite. I also re-enabled urql's test suite which now also works for Vite 3. Everything is green and no test suites are being skipped anymore. 🎉 Why did
|
I meant this repository uses two different versions and the vite-ecosystem-ci uses the same version. I now understand it's because of the
🎉
I found that vite-ecosystem-ci uses BTW I think this needs to be |
👍 Done: b8060ac.
It's set here: https://github.com/brillout/vite-plugin-ssr/blob/367a5b2a836cf43d1187074f4c1900c0abcd0c0e/.github/workflows/ci.yml#L68 Is there a way to detect whether the test is being run in vite-ecosystem-ci? I can then change the code accordingly. |
I see. I think just replacing |
Done 9e3ad05. I also think it should work now 👍. |
This PR applies some change to make vite-ecosystem-ci pass.
bump pnpm version (719d1d3)
This is not related to vite-ecosystem-ci.
I think this
packageManager
field is outdated. The lockfile uses lockfileVersion 5.4 but this is supported from pnpm 7.0.0.https://github.com/brillout/vite-plugin-ssr/blob/fe4ff6af483d03346eeef34904c6f3fca9dec161/pnpm-lock.yaml#L1
So I upgraded to 7.6.0.
set react-dom as peerDeps of @apollo/client (bc9591d)
examples/graphql-apollo-react
was failing.https://github.com/vitejs/vite-ecosystem-ci/runs/7462644762?check_suite_focus=true#step:7:478
This was because
require('react-dom/server')
inside@apollo/client
was resolved to react 18.1.0 but react 18.2.0 was used by Vite.I added
react-dom
to peerDep of@apollo/client
bypnpm.packageExtensions
. Maybe this should be fixed on package side? I'm not sure.fix type error in examples/layouts-react/.testRun.ts (f7257e3)
examples/layouts-react
had a type error.https://github.com/vitejs/vite-ecosystem-ci/runs/7462644762?check_suite_focus=true#step:7:591
I simply fixed it.
BTW is there any
tsconfig.json
which covers files under examples directory? IIUC if there isn't any, the tsconfig in vite-ecosystem-ci will be applied.Maybe the tsconfig in ecosystem-ci should exclude files in
workspace
directory so that it won't be applied to files which are cloned?set ssr.noExternal to graphql-apollo-vue (e7238e1)
examples/graphql-apollo-vue
was failing.https://github.com/vitejs/vite-ecosystem-ci/runs/7462644762?check_suite_focus=true#step:7:751
When building client,
@apollo/client
,@vue/apollo-composable
both resolves tomodule
field.When building SSR, these packages are externalized. But these packages does not have ESM entries, and these CJS entries are not compatible with node.js's automatic named export support.
So these packages needs to be bundled. I added them to
ssr.noExternal
.Also I seems something strange is happening around package overrides.
When you run
pnpm build
inexamples/graphql-apollo-vue
, the output is:So Vite 3 is used for client build and Vite 2 is used for SSR build. This maybe the reason why the CI is passing in this repository but not passing in ecosystem-ci.
(Not included in this PR) preact
Preact now supports Vite 3, so this code might be able to be removed after upgrading
@preact/preset-vite
to2.3.0
.https://github.com/brillout/vite-plugin-ssr/blob/fe4ff6af483d03346eeef34904c6f3fca9dec161/boilerplates/.testRun.ts#L38-L44