draft: merge webpack to vite #2371

rxliuli · 2022-04-22T05:43:49Z

changeset-bot · 2022-04-22T05:43:52Z

⚠️ No Changeset found

Latest commit: 1a0a3ef

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

acao · 2022-04-22T06:39:02Z

packages/graphiql/src/main.tsx

@@ -26,24 +26,28 @@ import './css/doc-explorer.css';
 import './css/history.css';


I'm not sure how this file ended up in src (maybe I moved it?) but it should not be part of the resulting build. It's meant to just be an example implementation. It should be example.js, and not be a typescript file. The bundler should not pick up this file

Yes, it seems that I didn't notice that tsc will compile all the files in the directory, and src/main.tsx is only used as the entry script of the web application (the official practice of the vite project). Probably should be excluded from tsc or moved to resources/ directory

ah, yes src/main.tsx should be src/example.js and should not be used as the bundling entry point, that should be src/cdn.js if you see in the webpack config. I originally had this file in the resources/ directory, it must have been moved during the tabs effort. src/index.js is the ESM main, which is different, and not what you want to treat as the entrypoint with vite.

Fixed, I excluded src/main.tsx from tsconfig.json/tsconfig.esm.json
1a0a3ef

this file should not be importing individual css files either, the idea is that it's supposed to show a demo using the CDN bundle and thus the css bundle... I need to clean this file up in another PR!

huh weird, webpack supports it for umd by just un-splitting the code and inlining the dynamic imports. this is going to be a big problem then.

the esm bundle users want dynamic imports for SSR, and the cdn users just want a single file and don't really need dynamic imports. there has to be a way? we originally were using inline require() statements when I inherited the project, as dynamic import was only a proposal then, but now esbuild can't handle that, so we switched to modern dynamic imports for codemirror/etc, which requires window to be present on import (thus why we can't import top-level)

@rxliuli it seems some of these solutions might work:

vitejs/vite#2982

I played around with this too recently. There's a option that inlines all dynamic imports:

export default defineConfig({ // ... build: { lib: { /* libarary mode */ }, rollupOptions: { external: ['react', 'react-dom'], output: { inlineDynamicImports: true }, }, }, });

codecov · 2022-04-22T06:42:19Z

Codecov Report

Merging #2371 (1c0b702) into main (2d91916) will decrease coverage by 0.98%.
The diff coverage is 73.64%.

❗ Current head 1c0b702 differs from pull request most recent head 1a0a3ef. Consider uploading reports for the commit 1a0a3ef to get more accurate results

@@            Coverage Diff             @@
##             main    #2371      +/-   ##
==========================================
- Coverage   65.70%   64.72%   -0.99%     
==========================================
  Files          85       81       -4     
  Lines        5106     5321     +215     
  Branches     1631     1703      +72     
==========================================
+ Hits         3355     3444      +89     
- Misses       1747     1873     +126     
  Partials        4        4

Impacted Files	Coverage Δ
packages/codemirror-graphql/src/hint.ts	`94.73% <ø> (ø)`
packages/codemirror-graphql/src/lint.ts	`100.00% <ø> (ø)`
packages/codemirror-graphql/src/results/mode.ts	`47.05% <ø> (ø)`
...kages/codemirror-graphql/src/utils/forEachState.ts	`100.00% <ø> (ø)`
packages/codemirror-graphql/src/utils/hintList.ts	`95.65% <ø> (ø)`
...ackages/codemirror-graphql/src/utils/info-addon.ts	`1.02% <ø> (ø)`
...ckages/codemirror-graphql/src/utils/mode-indent.ts	`0.00% <0.00%> (ø)`
packages/codemirror-graphql/src/variables/hint.ts	`89.70% <ø> (ø)`
packages/codemirror-graphql/src/variables/mode.ts	`79.48% <ø> (ø)`
packages/graphiql/src/utility/fillLeafs.ts	`5.33% <ø> (ø)`
... and 76 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a09a7fa...1a0a3ef. Read the comment docs.

acao · 2022-04-22T08:21:33Z

packages/graphiql/src/main.tsx

+ * - webpack dev server
+ */
+
+import React from 'react';


this file should not contain imports. this example was meant to be a CDN usage example, that tests the CDN example using the UMD without esm or any bundling. this example implementaiton is not a file for users to import, I hope that's clear. the vast amount of our users use the unpkg/jsdelivr bundle, not the esm import and a bundler. please see the comment at the top of this file

acao · 2022-04-22T08:28:17Z

packages/graphiql/vite.config.ts

+
+export default defineConfig({
+  plugins: [
+    // reactRefresh(),


Here's an explanation of how library mode works:

https://vitejs.dev/guide/build.html#library-mode

we need to use this, and specify react and react-dom as externals, so the shipped UMD bundle has parity and doesn't break existing implementations.

we need the demo script for development to use this UMD global, and not use imports, because the former is more widely used

I think it will help you a lot to just take some time to study the webpack configuration, and understand how users are consuming this bundle, and try to re-create the same with vite

acao · 2022-04-22T09:10:22Z

packages/graphiql/src/main.tsx

@@ -0,0 +1,188 @@
+/**


yeah, this file just needs to be removed from this PR. it should instead import renderGraphiQL.js as before, which does not contain any typescript, and uses the UMD global

acao · 2022-04-22T09:11:03Z

packages/graphiql/index.html

+  </head>
+  <body>
+    <div id="graphiql">Loading...</div>
+    <script src="./src/main.tsx"></script>


remove main.tsx and instead point this to renderGraphiQL.js as we were already doing. it will expect a UMD bundle w/ global GraphiQL, not an esm module. This UMD bundle gets at least 700,000 downloads a month (that's only the stats for jsdelivr, we don't have unpkg stats)!

the HTML file should look like this, to simulate the CDN setup:
https://github.com/graphql/graphiql/blob/eab257ba3e65e47625216fc3545b56426e0c4e61/packages/graphiql/resources/index.html.ejs

acao · 2022-04-26T08:37:12Z

I plan on coming back to this FYI! I have no wifi at home but now I'm at the office, so I may be able to put more time into helping you on this!

rxliuli · 2022-04-26T09:14:04Z

I plan on coming back to this FYI! I have no wifi at home but now I'm at the office, so I may be able to put more time into helping you on this!

I don't have much time before 05.01, work content must be delivered by the 30th(deadline), maybe I will have some time to deal with the 5.1-5.5 holiday

acao · 2022-04-26T19:13:47Z

If you like! Would it be ok if I made some commits this week if I have some time?

acao · 2022-07-28T20:48:28Z

Sorry it’s been a long time! But I would love to revive this PR and finish this up to help out the upstream graphiql2 and users who need more/ new formats. @jonathanawesome you may be interested in tinkering with this as well!

rxliuli · 2022-07-29T00:19:31Z

Sorry, I haven't been motivated to do this since using Apollo explorer
https://studio.apollographql.com/sandbox/explorer

shabab477 · 2022-08-01T08:11:19Z

Hello @acao any plans to revive this?

acao · 2022-08-01T08:26:02Z

yes @shabab477 I was hoping someone would! just have not had time but was hoping to get to it in the next month or so. I would welcome any PRs towards a move to pnpm and move all library builds to esbuild & vite, and upgrade webpack example(s) to 5 and update vite examples etc.

I would also be open to any monorepo tooling suggestions as well, rush, moonrepo, turbobuild, nx.dev, etc. if we can replace a monorepo-wide incremental build/watch dev experience like we partially have with typescript project references that would be awesome. We used to have project refs working entirely across the monorepo before i ntroduced vite to graphiql and graphiql/react in an awkward way. @thomasheyenbrock has done his best with this setup and made some improvements, but I would love to give him and other graphiql maintainers and contributors a much better experience!

acao · 2022-08-01T08:51:10Z

@rxliuli i hope it's ok if we re-open your PR for reference as we make the migration?

rxliuli · 2022-08-01T08:52:24Z

@rxliuli i hope it's ok if we re-open your PR for reference as we make the migration?

Of course, feel free to use

acao · 2022-08-01T08:57:32Z

@rxliuli thank you and thank you for this helpful draft PR! I'm happy you found a tool that works for you. GraphQL is the only mission!

acao · 2022-08-01T17:03:50Z

@shabab477 let me know if and when you'd like to take this on! I am happy to help in whatever capacity, and am happy to schedule a time to learn about the repo if you have question.

I just want to get a few more releases out to the LSP, and some fixes for graphiql@1 before i dive down this important but inevitably complex "rabbit hole" haha

shabab477 · 2022-08-01T18:16:31Z

@acao I went over the repo. I'm open to work this weekend after my "usual" work 😉

shabab477 · 2022-08-07T07:29:34Z

@acao i created a PR here but seems a lot has changed(hooks instead of class components) since this commit has been made. I will have resolve the conflicts.

dimaMachina · 2024-08-07T14:05:51Z

closed by #3679

feat: merge webpack to vite

7911591

rxliuli changed the title ~~feat: merge webpack to vite~~ draft: merge webpack to vite Apr 22, 2022

fix: Fix unit test

1c0b702

acao reviewed Apr 22, 2022

View reviewed changes

fix: Fixed erroneously compiling main.tsx also as lib

1a0a3ef

acao reviewed Apr 22, 2022

View reviewed changes

acao mentioned this pull request May 1, 2022

fix babel config for webpack build #2383

Merged

acao mentioned this pull request Jul 28, 2022

Cannot create an instance of GraphiQL when using requirejs #2598

Open

rxliuli closed this Jul 29, 2022

acao reopened this Aug 1, 2022

acao mentioned this pull request Aug 7, 2022

Monorepo Tooling SOS Roadmap #2639

Open

dimaMachina mentioned this pull request Aug 7, 2024

[2] GraphiQL x Vite v5 #3679

Merged

3 tasks

dimaMachina closed this Aug 7, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

draft: merge webpack to vite #2371

draft: merge webpack to vite #2371

rxliuli commented Apr 22, 2022

changeset-bot bot commented Apr 22, 2022 •

edited

Loading

acao Apr 22, 2022

rxliuli Apr 22, 2022

acao Apr 22, 2022

rxliuli Apr 22, 2022 •

edited

Loading

acao Apr 22, 2022

acao Apr 22, 2022

acao Apr 22, 2022 •

edited

Loading

acao Apr 22, 2022

thomasheyenbrock Apr 22, 2022

acao Apr 22, 2022

codecov bot commented Apr 22, 2022 •

edited

Loading

acao Apr 22, 2022 •

edited

Loading

acao Apr 22, 2022

acao Apr 22, 2022

acao Apr 22, 2022 •

edited

Loading

acao Apr 22, 2022 •

edited

Loading

acao Apr 22, 2022 •

edited

Loading

acao commented Apr 26, 2022

rxliuli commented Apr 26, 2022 •

edited

Loading

acao commented Apr 26, 2022

acao commented Jul 28, 2022

rxliuli commented Jul 29, 2022

shabab477 commented Aug 1, 2022

acao commented Aug 1, 2022 •

edited

Loading

acao commented Aug 1, 2022

rxliuli commented Aug 1, 2022

acao commented Aug 1, 2022

acao commented Aug 1, 2022 •

edited

Loading

shabab477 commented Aug 1, 2022

shabab477 commented Aug 7, 2022 •

edited

Loading

dimaMachina commented Aug 7, 2024

		@@ -26,24 +26,28 @@ import './css/doc-explorer.css';
		import './css/history.css';

draft: merge webpack to vite #2371

draft: merge webpack to vite #2371

Conversation

rxliuli commented Apr 22, 2022

changeset-bot bot commented Apr 22, 2022 • edited Loading

⚠️ No Changeset found

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rxliuli Apr 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acao Apr 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Apr 22, 2022 • edited Loading

Codecov Report

acao Apr 22, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acao Apr 22, 2022 • edited Loading

Choose a reason for hiding this comment

acao Apr 22, 2022 • edited Loading

Choose a reason for hiding this comment

acao Apr 22, 2022 • edited Loading

Choose a reason for hiding this comment

acao commented Apr 26, 2022

rxliuli commented Apr 26, 2022 • edited Loading

acao commented Apr 26, 2022

acao commented Jul 28, 2022

rxliuli commented Jul 29, 2022

shabab477 commented Aug 1, 2022

acao commented Aug 1, 2022 • edited Loading

acao commented Aug 1, 2022

rxliuli commented Aug 1, 2022

acao commented Aug 1, 2022

acao commented Aug 1, 2022 • edited Loading

shabab477 commented Aug 1, 2022

shabab477 commented Aug 7, 2022 • edited Loading

dimaMachina commented Aug 7, 2024

changeset-bot bot commented Apr 22, 2022 •

edited

Loading

rxliuli Apr 22, 2022 •

edited

Loading

acao Apr 22, 2022 •

edited

Loading

codecov bot commented Apr 22, 2022 •

edited

Loading

acao Apr 22, 2022 •

edited

Loading

acao Apr 22, 2022 •

edited

Loading

acao Apr 22, 2022 •

edited

Loading

acao Apr 22, 2022 •

edited

Loading

rxliuli commented Apr 26, 2022 •

edited

Loading

acao commented Aug 1, 2022 •

edited

Loading

acao commented Aug 1, 2022 •

edited

Loading

shabab477 commented Aug 7, 2022 •

edited

Loading