DRAFT: Migrate graphiql package to vite

[jonathanawesome]

This PR is the latest attempt to move away from Webpack and into Vite.

The Vite config turned out to be much more non-standard that I'd expected. Because we need to prep a minified bundle and a non-minified bundle (for the Netlify deployments), and because Vite's library build mode doesn't allow processing of the required index.html file, there's some hacky bits regarding the Vite configs that I'm not particularly fond of. That said, it mostly works.

I've left all of the webpack-related code in for now, except the build scripts, for reference.

Here are the important notes for reviewers:

Broken subscriptions/e2e

Most critical is an error originating from createFetcher and the usage of require for providing a user-friendly error when graphql-ws is not installed as a dependency (introduced in #2407). This will die in the browser when attempting to execute a subscription (screenshot below) and, most importantly, causes the e2e test run to fail. I haven't thought of a fix nor was I interested in cramming a fix into this PR without first reviewing as a group.

Removal of graphiql.min.css.map

I struggled for a short while to get Vite to cough up a minified css source map without a plugin, then decided to abandon. I don't think removing the css source map on the development Netlify build is that big of a deal, but I'm happy to be wrong upon review and find a solution.

Bundle file target directory

I've changed the target directory for the bundled files, moving them from the root of the package to /build. If this interferes with packaging it can be changed, but we'll need to perform some additional Vite magic because Vite expects an index.html to live in the root and we can't have the bundler overwriting it. The new index.html is a template that is used in development mode and to produce the dev.html and index.html files that are used for the Netlify deployments...the bulk of this PR deals with hacking around this file and Vite's requirements for it. Happy to be wrong about my implementation and adjust upon review, but I think I've got it as clean as could be.

Non ideal dev mode

The script for dev mode starts the e2e server, starts a Vite build mode to watch for changes to /components/GraphiQL.tsx and rebuild the bundle, and starts the Vite dev server. It's complicated and not as lightning fast as you might expect with Vite, but necessary unless we want to jump through hoops to adhere to the "standard" Vite library mode setup. Early on in this work I had Ladle running and pointed directly at the GraphiQL component file, and that worked great, but it seemed even more awkward than the current state without a more well thought out approach to UI development throughout the repository. Ideally, we'll bring back Ladle or Storybook at some point in the near future to cover all of the UI development needs for UI-related packages and leave those packages to just handle bundling. @thomasheyenbrock has the most recent experience here and, I assume, was dev'ing /graphiql-react via graphiql...is that assumption correct?

---

Here's what I've got for a punch list:

• [ ] Find a solution for the createFetcher/require situation
• [ ] Come to a decision about the removal of graphiql.min.css.map
• [ ] Come to a decision about the modified bundle target directory
• [ ] Remove unused Webpack-related code
• [ ] Optional: use this opportunity to remove other old and unused build/tooling related code

[changeset-bot[bot]]

⚠️ No Changeset found

Latest commit: cf7b137056a70ee2d49d034351aefccea73c0f3e

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[github-actions[bot]]

The latest changes of this PR are not available as canary, since there are no linked changesets for this PR.

[acao]

this gets us so much closer to fixing the build vs build-bundles issue! now, i'm going to just rename some of these plugins to use build-bundles with wsrun -s so it executes in order of dependencies.

then build can be back to pure tsc --build project references, for all the type checks I see for underlying graphiql plugins etc

[acao]

@jonathanawesome in fact... thanks to this PR, I've figured out how to consolidate all of the graphiql packages & plugins into one consolidated vite config for the entire monorepo, by automatically resolving rollupOptions.entry and rollupOptions.output and/or with lib.entry https://github.com/vitejs/vite/discussions/1736#discussioncomment-3812904. We can generate all of the entry/output config from the package names at build time.

I built a bunch of esbuild plugins at work to build our own dev server because our monorepo was too complex for vite, but this is a case where we have multiple options to push vite further and make it more monorepo friendly.

[jonathanawesome]

@jonathanawesome in fact... thanks to this PR, I've figured out how to consolidate all of the graphiql packages & plugins into one consolidated vite config for the entire monorepo, by automatically resolving rollupOptions.entry and rollupOptions.output and/or with lib.entry vitejs/vite#1736 (comment). We can generate all of the entry/output config from the package names at build time.

That's fantastic!

[acao]

Oof. So it may be that this pattern for dev/prod builds will need to continue. Worth noting this is possible in webpack 5, technically it's also possible in webpack 4 by returning two configs as an array, but I have the env flag split approach there as well. Also this is possible with rollup. Oh... and it might be possible with turbopack as well! 🤔

this issue prevents what I was hoping to achieve, as well as how minify works globally like you said

However everything else - harmony between the build and build-bundles and monorepo wide dev server will still be doable!

[acao]

i think this could be our happy place... 😻 this would be a deus ex machina if it can work for us... it would solve the build vs build-bundles riddle transparently, and is sufficient enough for me to replace typescript project references. turborepo runs up against some of my preferences (per-workspace scripts for example) but we can add some plop generators for adding new workspaces (graphiql plugin or generic for starters)

https://github.com/vercel/turbo/blob/b7bcfc312e41367e2ec7f5fb37a6ee88a6b6545a/examples/kitchen-sink/package.json#L6

we have many options for self-hosting turborepo remote caches now (if we need that? i think we would benefit). for example: https://github.com/cometkim/turbocache (@cometkim is so prolific! hello there! 👋🏻 )

i think turborepo/turbopack/tsup could even perhaps replace esbuild for bundling our vscode extensions as well, and if we ever ship a standalone executable for the lsp cli, then turbprepo could even perhaps help with cacheing for that?

[cometkim]

Heya! :wave: @acao turbocache is a very simple demo project showing how to host a turborepo remote cache on Cloudflare Workers, so depending on your use case you may need to customize it, but it's very easy!

If you're looking for a bundler for libraries/binary that doesn't use css or html entries, you can keep watching at nanobundle. Very similar to microbundle, but uses esbuild. I'm are working on adding multi-entry support.

[acao]

turborepo uses tsup in many examples which uses both swc and esbuild?

really, anywhere that tsup is used we could nanobundle, and the options are almost the same? for example --dts flag. haha interesting

we can also still use vite library mode with turborepo as well!

[acao]

@cometkim I don't think we need multi-entrypoint support actually if we can use turborepo. i think we can use per-workspace builds if we just use nx or turborepo or something similar to orchestrate per-workspace build and dev scripts more intelligently than we are now, and then replace tsc --build with cross-monorepo watch build.

i think nanobundle will suit us just fine as is for many cases (except for the few with css)! It has dts support, and support for multiple output formats, and many configuration options for the cases where we might need them, which is all we really need.

one note: to replace what tsup would do for us here, we would need to add --watch flag to nanobundle - I recently worked on an internal project where we made a custom watcher for esbuild if that is necessary! this doesn't need to be an all out dev server, just replace tsc --watch

[acao]

i hope to return to this effort next week!

[acao]

I've got it working again on a new branch by plucking these changes onto the current main with git checkout. vite has since released a 4.x

Vite expects an index.html to live in the root and we can't have the bundler overwriting it.

agreed! this is how graphiql has always worked as well, at least in terms of where the index.html is always written to. The source being there is funky. we can publish the demo from any directory, netlify isn't required to publish packages/graphiql as the root, but we do need to make sure graphiql.min.js and .css and such are still at least aliased to the root import paths for unpkg/jsdelivr etc

one issue I've found is that @graphiql/react is using vite@2 and graphiql itself is using [email protected]! going to rectify all of these versions and create some harmony :))

[acao]

I've managed to address the require() issue in toolkit, bundling the library itself in a compatible way. Now something is causing vite to not encode this unicode character properly in reach ui, have you seen this before?

https://github.com/reach/reach-ui/blob/43f450db7bcb25a743121fe31355f2294065a049/packages/popover/src/reach-popover.tsx#L338

[malte-behrendt-gr]

I've managed to address the require() issue in toolkit, bundling the library itself in a compatible way. Now something is causing vite to not encode this unicode character properly in reach ui, have you seen this before?

https://github.com/reach/reach-ui/blob/43f450db7bcb25a743121fe31355f2294065a049/packages/popover/src/reach-popover.tsx#L338

@acao Just to clarify: is the issue that something does not get rendered or that the variable name, restoreTabIndexTuplés, contains a unicode character?

If the first: do you have a link to a error message? If the latter, is a normal 'e' not usable?

[Sytten]

What there ever a fix for the require, I am trying to use the package and its PITA

EDIT: Found a way! Add the following to your vite config.

build: {
    commonjsOptions: {
      transformMixedEsModules: true,
    },
  },

[dimaMachina]

closed by https://github.com/graphql/graphiql/pull/3679