Page MenuHomeElementl

[dagit] Eject CRA
ClosedPublic

Authored by dish on Apr 13 2021, 9:54 PM.

Details

Summary

Eject the Dagit create-react-app.

So far, we've been working around CRA in order to get a good dev/build workflow in place with the yarn workspace. I hacked in some babel/ts watching to create an output for @dagit/core that the CRA could observe, but it's pretty fragile. It also seems to prevent us from code splitting, since the JS is already built by the time the CRA build runs.

Instead, let's just go ahead and eject. It doesn't seem like the react-scripts project is moving so quickly that we can't keep up as needed, and this gives us total control over our Webpack configuration without sacrificing HMR and other nice features built into CRA. (I spent some time looking into forking CRA, as suggested by the CRA docs, but it was a pretty serious pain to get things working locally, and I don't think our needs in @dagit/app are identical to elsewhere anyway.)

This diff ejects the CRA and adds one item to Webpack: @dagit/core will now be included alongside @dagit/app src in the build. This means:

  • Changes to @dagit/core will trigger HMR, without anything special
  • I can remove all my babel/ts watching hackery
  • VSCode click-to-code actually goes to the code instead of the generated ts files
  • The @dagit/core main file can just be its tsx entry point, so we get its TS types without emitting d.ts files
  • We're positioned to start doing code splitting
  • We have flexibility to do other things that are currently difficult with CRA, like adding other webpack loaders

I added DAGSTER to comments where I have made changes. I'll comment inline. Pretty much everything else in the diff is CRA ejecta.

Test Plan

Dev workflow:

  • Run dagit. Run make dev_webapp from js_modules/dagit, verify that the app builds.
  • Navigate to "Runs" in Dagit. Make a change to RunsRoot, verify that HMR is triggered and that the code change appears immediately in the browser.
  • Introduce lint and TS errors, verify that the error overlay appears correctly.

Prod workflow:

  • Run yarn build-for-python from js_modules/dagit. Verify successful build result.
  • Run dagit, open browser. Verify that everything looks correct.
  • Run dagit with path prefix. Verify same.

Diff Detail

Repository
R1 dagster
Branch
dish-eject
Lint
Lint Passed
Unit
No Test Coverage

Event Timeline

dish created this revision.

Roll back a change I don't need yet.

js_modules/dagit/packages/app/config/paths.js
75–76

Path to dagit, for use in Webpack config.

js_modules/dagit/packages/app/config/webpack.config.js
386–387

Previously just paths.appSrc, now includes paths.dagitCore.

js_modules/dagit/packages/app/package.json
6

I assume this is accurate?

js_modules/dagit/packages/app/src/typings.d.ts
38

Copied from @dagit/core custom types, which makes the @dagit/app tsconfig much simpler.

dish requested review of this revision.Apr 13 2021, 10:06 PM

Rebase, squash a rogue commit

This revision is now accepted and ready to land.Apr 15 2021, 4:10 AM
This revision was automatically updated to reflect the committed changes.