Page MenuHomeElementl

[dagit] Split app into packages
ClosedPublic

Authored by dish on Mar 26 2021, 4:57 PM.

Details

Summary

This diff divides the Dagit app into yarn workspaces with their own packages. This gives us flexibility to break Dagit source apart further, treating it more like a library.

The bulk of the diff is the movement of files from dagit/src to dagit/packages/core/src, and can be ignored.

The dagit directory now just contains a package.json that defines these two workspaces and a couple yarn scripts. The workspaces are:

  • @dagit/core, at dagit/packages/core
    • Exports a couple of root components (for now), contains 99.5% of Dagit source
    • Uses babel and tsc builds to produce js and d.ts output for consumption
    • Includes a couple of yarn scripts to start watchers, enabling HMR-enabled development of Dagit source while working on the Dagit app
  • @dagit/app, at dagit/packages/app
    • A thin CRA wrapper that just renders components from @dagit/core, for now. Assume this will become more complex.

Each package has its own ts and lint scripts.

I have also made some changes to dagit/Makefile.

For the most part, the dev and build workflow should remain the same as they are. You'll still run the same make commands to start the app in development mode or for a build. A major change here is that the start command (for dev) will now also kick off the watchers for @dagit/core, so that the app will build and update correctly while you develop it.

Test Plan

Within both packages/core and packages/app, run yarn ts and yarn lint. Also run yarn download-schema and yarn generate-types within packages/core to verify that these work correctly.

Development:

  • Run make dev_webapp from js_modules/dagit. Verify that the app build starts and watches correctly, and that once everything is working, my changes to @dagit/core code correctly trigger rebuilds and HMR in the browser.
  • Run yarn build-for-python from js_modules/dagit. Verify that a full prod build is created and put in the correct python_modules destination.

The above is also verified with the current Buildkite config.

Diff Detail

Repository
R1 dagster
Lint
Lint Not Applicable
Unit
Tests Not Applicable

Event Timeline

Run initial dagit/core install before app ts

Harbormaster returned this revision to the author for changes because remote builds failed.Mar 26 2021, 5:41 PM
Harbormaster failed remote builds in B28052: Diff 34384!
Harbormaster returned this revision to the author for changes because remote builds failed.Mar 26 2021, 6:43 PM
Harbormaster failed remote builds in B28058: Diff 34392!
Harbormaster returned this revision to the author for changes because remote builds failed.Mar 26 2021, 8:08 PM
Harbormaster failed remote builds in B28061: Diff 34396!
dish requested review of this revision.Mar 26 2021, 8:55 PM

Comments on a handful of the files here. Will update the summary and test plan.

.buildkite/dagster-buildkite/dagster_buildkite/steps/dagit.py
13

coverage now lives in packages/core.

js_modules/dagit/Makefile
20

Updated all of these. Some we don't need anymore IMO, and others should now point at the workspace commands.

js_modules/dagit/package.json
19

The stripped-down project root. Defines our workspaces and just a couple of scripts to be run for development and build.

js_modules/dagit/packages/app/README.md
1 ↗(On Diff #34405)

Will delete this.

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

The thin CRA. Doesn't have to do much, just render AppProvider and App.

js_modules/dagit/packages/app/public/logo512.png
1 ↗(On Diff #34405)

I'll delete these.

js_modules/dagit/packages/app/src/index.tsx
11

The Dagit CRA.

js_modules/dagit/packages/core/package.json
5 ↗(On Diff #34405)

We now build JS files (with babel) and TS types with (tsc) and point @dagit/app at them.

125 ↗(On Diff #34405)

react and react-dom are peer deps to avoid conflicts with CRA.

js_modules/dagit/packages/core/src/main.tsx
4 ↗(On Diff #34405)

The top-level export for @dagit/core. This can become as complex and granular as needed.

js_modules/dagit/tox.ini
33

Updated to use workspace commands.

python_modules/dagit/dagit/app.py
146 ↗(On Diff #34405)

This can be broken out into a separate diff. It allows the app to be created from an arbitrary path, passed in as target_dir.

dish edited the test plan for this revision. (Show Details)
dish added reviewers: bengotow, max, dgibson, prha.

Split off app.py change into its own diff

Split code moves into their own diff

seems reasonable to me. Any thoughts or objections @bengotow ?

This looks great, I pulled it down and ran all the scripts etc - all worked smoothly! 🙌

This revision is now accepted and ready to land.Mar 29 2021, 3:12 PM
This revision was automatically updated to reflect the committed changes.