Step 3: convert browserify build to webpack, fixes #976 #1001

acao · 2019-11-18T12:38:59Z

the vast amount the added lines to this PR are via the yarn.lock file, FYI! this PR is mostly a 120 line webpack config file and some tooling and configuration changes.

browserify min and graphiql bundle replaced with webpack 3, retaining window.GraphiQL interface pattern
windows compatible tooling: replace bash scripts for babel, browserify, uglify, postcss/autoprefixer, etc 'build' turned into build 20s and build-bundle. This came with replacing browserify, as we no longer needed a bash script. Also added crossenv, rimraf, etc
use new webpack mincss plugin, and generate a nanocss build at graphiql.min.css too! (note: temporary til styled components)
update yarn dev server to mount webpack dev server with test GraphQL server, instead of the exact same pattern using browserify
fix some issues with cypress + jest battling over the linting of expect() in cypress specs that was failing lint
HUGE reduction in minified build size. I'm not kidding.

(parsed size, from webpack output)

here is proof (on the far right) that the file utility from the first PR in this series is working:

packages/graphiql/package.json

codecov · 2019-11-18T22:54:29Z

Codecov Report

Merging #1001 into master will increase coverage by 0.21%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master    #1001      +/-   ##
==========================================
+ Coverage   43.52%   43.73%   +0.21%     
==========================================
  Files          66       65       -1     
  Lines        2996     3018      +22     
  Branches      649      656       +7     
==========================================
+ Hits         1304     1320      +16     
- Misses       1421     1425       +4     
- Partials      271      273       +2

Impacted Files	Coverage Δ
packages/graphiql/src/components/QueryHistory.js	`36.53% <ø> (ø)`	⬆️
packages/graphiql/src/components/DocExplorer.js	`38.29% <ø> (ø)`	⬆️
packages/graphiql/src/components/QueryEditor.js	`57.94% <0%> (+3.18%)`	⬆️

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 0b93b55...4c46196. Read the comment docs.

packages/graphiql/test/dev-server.js

packages/graphiql/cypress/plugins/index.js

acao · 2019-12-02T08:01:30Z

packages/graphiql/package.json

@@ -64,27 +67,32 @@
    "react-dom": "^15.6.0 || ^16.0.0"
  },
  "devDependencies": {


replacing the entire bundling toolchain. thus the huge yarn lock diff

benjie

LGTM; if you're happy it builds correctly then I'm happy.

package.json

benjie · 2019-12-02T18:29:52Z

packages/graphiql/cypress/support/commands.js

  cy.window().then(w => {
    cy.expect(JSON.parse(w.g.resultComponent.viewer.getValue())).to.deep.equal(
      mockSuccess,
    );
-  });
+  })


Lint issue?

benjie · 2019-12-02T18:47:01Z

babel.config.js

+  envConfig.modules = 'umd'
+  envConfig.targets = null
+  envConfig.ignoreBrowserslistConfig = false
+}


I'd rather this used an if/elseif/else chain and instead of overwriting the initial properties of envConfig it set variables that were then used to create the envConfig object afterwards, but 🤷‍♂️

I'm so averse to else it's problematic, haha! will take note for further iterations

Co-Authored-By: Benjie Gillam <benjie@jemjie.com>

caseywebdev · 2019-12-09T18:32:47Z

packages/codemirror-graphql/.npmignore

@@ -0,0 +1,4 @@
+babel.config.js
+.gitignore
+resources


By not publishing resources here, the reference at

graphiql/packages/graphiql/src/components/ResultViewer.js

Line 45 in 98be6f5

require('codemirror-graphql/results/mode');

is no longer valid. Either this line or that line should be removed.

I’m confused? that import is from codemirror-graphql, which still exports that module. are you having issues? this PR was merged

if what you are saying is true, then the results viewer would fail to highlight jaon here in the current build of master:

https://graphiql-test.netlify.com

Sorry, I got resources and results confused so this isn't the direct issue. Regardless, there's still an issue with the latest codemirror-graphql package because the results folder, referenced in the line above, does not exist.

~ npm install codemirror-graphql@0.11.5 ~ ls node_modules/codemirror-graphql/results ls: node_modules/codemirror-graphql/results: No such file or directory

This broken link is causing my build to fail, so I've resorted back to graphiql@0.17.2 and manually pinning codemirror-graphql@0.11.4 (which does publish this folder) for now.

@caseywebdev try now with 0.11.6

https://unpkg.com/browse/codemirror-graphql@0.11.6/ my bad! we should be doing pre-releases for all these build changes.

Working again 👍🏻 Thanks for the quick turnaround!

acao · 2019-12-09T18:55:37Z

oh yikes! thanks for reporting this. ill have to see what happened and fix it immediately

…

On Mon, Dec 9, 2019 at 1:46 PM Casey Foster ***@***.***> wrote: ***@***.**** commented on this pull request. ------------------------------ In packages/codemirror-graphql/.npmignore <#1001 (comment)>: > @@ -0,0 +1,4 @@ +babel.config.js +.gitignore +resources Sorry, I got resources and results confused so this isn't the direct issue. Regardless, there's still an issue with the latest codemirror-graphql package because the results folder, referenced in the line above, does not exist. ~ npm install ***@***.***~ ls node_modules/codemirror-graphql/results ls: node_modules/codemirror-graphql/results: No such file or directory This broken link is causing my build to fail, so I've resorted back to ***@***.*** and manually pinning ***@***.*** (which does publish this folder) for now. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#1001?email_source=notifications&email_token=AAKOFF2YWIUCAVC6KMZOBPTQX2G65A5CNFSM4JOS5R2KYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCOPD3BQ#discussion_r355618948>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAKOFF3NESQO2WQCCOGX4ZLQX2G65ANCNFSM4JOS5R2A> .

acao changed the title ~~ci: convert browserify to webpack, fixes #976~~ Step 3: convert browserify to webpack, fixes #976 Nov 18, 2019

acao changed the title ~~Step 3: convert browserify to webpack, fixes #976~~ Step 3: convert browserify build to webpack, fixes #976 Nov 18, 2019

acao force-pushed the chore/replace-browserify-webpack branch 3 times, most recently from 17e85c6 to 03bfcad Compare November 18, 2019 17:47

kevinsimper reviewed Nov 18, 2019

View reviewed changes

packages/graphiql/package.json Outdated Show resolved Hide resolved

acao changed the title ~~Step 3: convert browserify build to webpack, fixes #976~~ [WIP] Step 3: convert browserify build to webpack, fixes #976 Nov 18, 2019

acao force-pushed the chore/replace-browserify-webpack branch from 03bfcad to 48452c0 Compare November 18, 2019 19:51

acao changed the title ~~[WIP] Step 3: convert browserify build to webpack, fixes #976~~ Step 3: convert browserify build to webpack, fixes #976 Nov 18, 2019

acao force-pushed the chore/replace-browserify-webpack branch 8 times, most recently from 0961e68 to e0a01ea Compare November 19, 2019 01:03

acao force-pushed the chore/esm-build branch from f97c273 to 3bed71b Compare November 19, 2019 01:23

acao force-pushed the chore/replace-browserify-webpack branch 3 times, most recently from 15df49b to 275a5f2 Compare November 19, 2019 17:32

acao force-pushed the chore/esm-build branch 8 times, most recently from 1538a0c to 915a595 Compare November 20, 2019 03:23

acao commented Nov 27, 2019

View reviewed changes

packages/graphiql/test/dev-server.js Show resolved Hide resolved

acao commented Nov 27, 2019

View reviewed changes

packages/graphiql/cypress/plugins/index.js Show resolved Hide resolved

acao force-pushed the chore/esm-build branch from 3c99618 to 7021e10 Compare November 30, 2019 14:52

acao changed the base branch from chore/esm-build to master November 30, 2019 15:11

acao and others added 7 commits November 30, 2019 10:14

chore: esm for all packages

6d87b1b

ci: convert browserify to webpack, fixes #976

0d9b9c2

chore: remove vendor files for react/etc

9e5e0f4

chore: fix webpack targets for minified build

bf925d1

ci: add bundle step to netlify

1b0c256

chore: use cross-env for every script that uses env

3d0a989

fix: fix markdown-it bug by force-adding the entities version

fa5e372

acao force-pushed the chore/replace-browserify-webpack branch from 328a0b0 to fa5e372 Compare November 30, 2019 15:18

acao requested review from benjie and orta December 2, 2019 01:12

acao mentioned this pull request Dec 2, 2019

Bundle size? #622

Closed

acao added the graphiql label Dec 2, 2019

acao commented Dec 2, 2019

View reviewed changes

benjie approved these changes Dec 2, 2019

View reviewed changes

chore: simpler cypress-open script

4c46196

Co-Authored-By: Benjie Gillam <benjie@jemjie.com>

acao merged commit 3caf041 into master Dec 3, 2019

caseywebdev reviewed Dec 9, 2019

View reviewed changes

acao deleted the chore/replace-browserify-webpack branch December 28, 2019 22:21

acao added this to the GraphiQL-1.0.0-beta milestone Mar 14, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Step 3: convert browserify build to webpack, fixes #976 #1001

Step 3: convert browserify build to webpack, fixes #976 #1001

acao commented Nov 18, 2019 •

edited

Loading

codecov bot commented Nov 18, 2019 •

edited

Loading

acao Dec 2, 2019

benjie left a comment

benjie Dec 2, 2019

benjie Dec 2, 2019

acao Dec 2, 2019

caseywebdev Dec 9, 2019

acao Dec 9, 2019

acao Dec 9, 2019

caseywebdev Dec 9, 2019

acao Dec 9, 2019

acao Dec 9, 2019

caseywebdev Dec 9, 2019

acao commented Dec 9, 2019 via email

Step 3: convert browserify build to webpack, fixes #976 #1001

Step 3: convert browserify build to webpack, fixes #976 #1001

Conversation

acao commented Nov 18, 2019 • edited Loading

codecov bot commented Nov 18, 2019 • edited Loading

Codecov Report

Choose a reason for hiding this comment

benjie left a comment

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

acao commented Dec 9, 2019 via email

acao commented Nov 18, 2019 •

edited

Loading

codecov bot commented Nov 18, 2019 •

edited

Loading