search

richardtallent / vite-plugin-singlefile

Vite plugin for inlining JavaScript and CSS resources
MIT License
808 stars 53 forks source link

ci: fix CI for Node v14 by updating npm #65

Closed aloisklink closed 1 year ago

aloisklink commented 1 year ago

It looks like I was wrong about https://github.com/richardtallent/vite-plugin-singlefile/pull/64#discussion_r1019713613 (my fault, I should have tested it), the CI didn't actually work in Node v14 due to a different reason:

NPM versions before 7 do not automatically install "peerDependencies" when running npm install locally, so do not work with this repo (this behaviour is also the default for yarn)

Because the GitHub actions/setup-node@v3 comes with NPM v6 for Node v14, this means we need to manually upgrade to NPM@>=7 for CI to run.

Design Decisions

It's also possible to fix this issue by copying your "peerDependencies" to "devDependencies", to support npm@6 and yarn, but considering they're only needed for local/development installs, it's probably not worth the effort to support them!

richardtallent commented 1 year ago

Thanks, I've merged this in. It's still unlikely that the test will pass for Node 14, since Vue 3 (the framework I use, and thus the one vite-plugin-singlefile-example is build on) requires Node 16.

aloisklink commented 1 year ago

It's still unlikely that the test will pass for Node 14, since Vue 3 (the framework I use, and thus the one vite-plugin-singlefile-example is build on) requires Node 16.

It seems to be working fine, but that might just be random-luck because the example project doesn't use any Node.JS v16 specific code in Vue (but future Vue versions might break this)!

If the vite-plugin-singlefile-example ever does fail, it should be easy to remove 14 from the following line https://github.com/richardtallent/vite-plugin-singlefile/blob/03d601f012d4e77170c395c378a724d137844ffd/.github/workflows/test.yml#L35

After all, the main purpose of CI is to make the programmer's life easier by automating tests. If it becomes too hard to fix CI-only issues, it might just be easier to get rid of CI :)