Streamline vite configs

HerbCaudill commented 3 months ago

Turns out the vite config that we've been copy-pasting all over the place can be streamlined considerably without breaking anything.

The topLevelAwait plugin is no longer necessary
The exclusions in optimizeDeps now cause the automerge-wasm package not to be bundled at all. Not sure what changed, maybe we were working around a bug that got fixed upstream.
Nobody remembers for sure why we had server.fs.strict = false but maybe it was necessary to get the wasm to load at some point.

This PR updates all the vite.config.ts files in the monorepo. It also amends the ancestral config in the README walkthrough, and removes vite-plugin-top-level-await from dependencies.

Testing

pnpm install
pnpm build
pnpm test

Confirm that the react todo demo runs:

pnpm dev

Confirm that the svelte counter demo runs:

pnpm dev:svelte-demo

Confirm that the use-awareness example runs (and incidentally that the hooks package build is good)[^*]

pnpm -F automerge-use-awareness-example-project dev

Confirm that the react counter example runs:

pnpm -F @automerge/automerge-repo-demo-counter dev

~~Confirm that create-vite-app runs:~~

cd ˜/somewhere
pnpm create @automerge/create-vite-app hello-automerge
cd hello-automerge
pnpm install
pnpm dev

[^*]: depends on #321

pvh commented 3 months ago

@HerbCaudill how would the create-vite-app test you suggest there pick up the local copy? ISTM it would pull from npm

HerbCaudill commented 3 months ago

@HerbCaudill how would the create-vite-app test you suggest there pick up the local copy? ISTM it would pull from npm

uhhhh yeah you're right

HerbCaudill commented 3 months ago

@alexjg how would you suggest testing this? Is there a way to invoke npm create with reference to the local package?

alexjg commented 3 months ago

Getting npm create to actually recognise the package is extremely gnarly. However, all that npm create actually does is run the binary referred to by the create-* package. To test the output of the package then:

Run yarn build in the create-* package
Run npm pack in the create-* package to generate a tarball containing the create-* package (note that yarn pack fails to pick up the template directory when packing for reasons I don't understand).
Create a temporary directory
In the temporary directory run yarn init
In the temporary directory run yarn install <path to tarball created in2>
In the temporary directory run ./node_modules/@automerge/create-vite-app/dist/index.js someapp

Now the <tempoarary directory>/someapp directory contains the output of the create script, which we can assert things against.

This is a lot of schlep, I would love to hear about a better way.

alexjg commented 3 months ago

Theoretically you could dispense with the whole npm pack bit, but a significant part of the package is the template which is shipped with it and npm pack checks that the template is actually included correctly

pvh commented 3 months ago

(We should write a shell script for testing this)

On Thu, Mar 21, 2024 at 06:48 alexjg @.***> wrote:

Getting npm create to actually recognise the package is extremely gnarly. However, all that npm create actually does is run the binary referred to by the create-* package. To test the output of the package then:

Run yarn build in the create-* package

Run npm pack in the create- package to generate a tarball containing the create- package (note that yarn pack fails to pick up the template directory when packing for reasons I don't understand).

Create a temporary directory

In the temporary directory run yarn init

In the temporary directory run yarn install <path to tarball created in 2>

In the temporary directory run .@.***/create-vite-app/dist/index.js someapp

Now the /someapp directory contains the output of the create script, which we can assert things against.

This is a lot of schlep, I would love to hear about a better way.

— Reply to this email directly, view it on GitHub https://github.com/automerge/automerge-repo/pull/322#issuecomment-2012354463, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAAAWQGTE3OUFNT5JWGCBFLYZLQLBAVCNFSM6AAAAABE763ZN6VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDAMJSGM2TINBWGM . You are receiving this because you commented.Message ID: @.***>

HerbCaudill commented 3 months ago

(We should write a shell script for testing this)

I've made a node script to do a smoke test for create-vite-app.

⚡︎ pnpm -C packages/create-vite-app test

> @automerge/create-vite-app@1.1.4 test /Users/herbcaudill/Code/automerge/automerge-repo/packages/create-vite-app
> node test.js

building create-vite-app...
creating tarball...
creating test app...
installing test app...
running test app in dev mode...
✅ create-vite-app test passed

Following @alexjg's instructions above, it creates a vite app, then it runs dev against it and verifies that it started. There are probably other things that would be useful to verify, but this at least confirms that the vite config changes didn't break anything.

acurrieclark commented 3 months ago

@HerbCaudill i am waiting to merge my svelte patch until this one is merged as I think the vote config will conflict slightly. Do you think this is ready to land?

HerbCaudill commented 3 months ago

Do you think this is ready to land?

Should be, I'd suggest running the test script I just added to check my work & if all looks good feel free to merge.

pvh commented 3 months ago

Merged! Will ship along with a few other things in 1.1.6.

automerge / automerge-repo

Streamline vite configs #322

Testing