feat: workbench dev server#770
Conversation
|
Claude finished @joshuaellis's task in 1m 54s —— View job Code Review
Missing unit tests for orchestration logic.
The tests for
|
Coverage Delta
Comparing 6 changed files against main @ Overall Coverage
|
Coverage Delta
Comparing 4 changed files against main @ Overall Coverage
|
|
@joshuaellis First iteration: what do you think about the approach? |
Coverage Delta
Comparing 4 changed files against main @ Overall Coverage
|
|
@joshuaellis Added a new branch |
joshuaellis
left a comment
There was a problem hiding this comment.
Good start on this, we probably need the federation package to be ready so we can change the runtime static file the dev/build commands create if federation is enabled & therefore render a shell for MF to pull in the workbench remotely?
Coverage Delta
Comparing 5 changed files against main @ Overall Coverage
|
Coverage Delta
Comparing 6 changed files against main @ Overall Coverage
|
Coverage Delta
Comparing 6 changed files against main @ Overall Coverage
|
Coverage Delta
Comparing 6 changed files against main @ Overall Coverage
|
Coverage Delta
Comparing 6 changed files against main @ Overall Coverage
|
Coverage Delta
Comparing 6 changed files against main @ Overall Coverage
|
Coverage Delta
Comparing 6 changed files against main @ Overall Coverage
|
Preview this PR with pkg.pr.newRun the Sanity CLInpx https://pkg.pr.new/sanity-io/cli/@sanity/cli@f5f0dac <command>...Or upgrade project dependencies📦
|
| try { | ||
| moduleResolve('sanity/workbench', pathToFileURL(path.join(workDir, 'package.json'))) | ||
| workbenchAvailable = true | ||
| } catch { | ||
| // sanity/workbench not available in this version — skip workbench server | ||
| } |
There was a problem hiding this comment.
since we're re-exporting @sanity/workbench from sanity do we need these checks anymore? do they do this for studio servers?
There was a problem hiding this comment.
Imo we do need this check, because otherwise if dev is running with a version of sanity that does not export sanity/workbench the workbench dev-server would start, but couldn't render anything, because the export does not exist.
This ensures when the export does not exist, do not start the workbench dev-server and fallback to the current behavior (application dev-server is printed).
Does that make sense?
There was a problem hiding this comment.
But when do you think thats gonna happen? My understanding is we dont encourage anyone to actually install the cli manually, it comes from the sanity package incl. all the commands so i'm just wondering if we're guarding for a situation that won't happen?
There was a problem hiding this comment.
For now this is a guard for ourselves, if we are running the CLI with e.g. a stable studio release. We could also do a hard fail in that case?
For the future: it depends - once we release this PR, workbench can only work, when the sanity package has the export. If users are running an old studio e.g. it will fail.
This may also happen if someone has a global old version of the CLI installed.
32d91a5 to
d1b9a69
Compare
Coverage Delta
Comparing 6 changed files against main @ Overall Coverage
|
|
We don't want to have people install workbench directly. At the moment this follow a similar pattern to how people configure their cli as well where the studio re-exports the configuration function from the cli. |
d1b9a69 to
ebc0fad
Compare
Coverage Delta
Comparing 6 changed files against main @ Overall Coverage
|
Coverage Delta
Comparing 6 changed files against main @ Overall Coverage
|
12b2d3e to
b7b69c8
Compare
7e0d80c to
b59aa5c
Compare
📦 Bundle Stats —
|
| Metric | Value | vs feat/workbench (b7b69c8) | vs v6.3.1 |
|---|---|---|---|
| Internal (raw) | 2.1 KB | - | - |
| Internal (gzip) | 799 B | - | - |
| Bundled (raw) | 11.06 MB | - | +9.12 MB, +470.6% |
| Bundled (gzip) | 2.07 MB | - | +1.60 MB, +335.7% |
| Import time | 918ms | +2ms, +0.2% | +16ms, +1.7% |
bin:sanity
| Metric | Value | vs feat/workbench (b7b69c8) | vs v6.3.1 |
|---|---|---|---|
| Internal (raw) | 975 B | - | - |
| Internal (gzip) | 460 B | - | - |
| Bundled (raw) | 9.83 MB | - | +9.12 MB, +1286.7% |
| Bundled (gzip) | 1.77 MB | - | +1.60 MB, +940.0% |
| Import time | NaNs | - | NaNs, NaN% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
📦 Bundle Stats — @sanity/cli-core
Compared against feat/workbench (b7b69c82) · v1.3.0 (npm)
| Metric | Value | vs feat/workbench (b7b69c8) | vs v1.3.0 |
|---|---|---|---|
| Internal (raw) | 92.2 KB | - | - |
| Internal (gzip) | 21.6 KB | - | - |
| Bundled (raw) | 21.64 MB | - | +9.12 MB, +72.9% |
| Bundled (gzip) | 3.43 MB | - | +1.59 MB, +86.9% |
| Import time | 855ms | -11ms, -1.3% | +45ms, +5.6% |
🗺️ View treemap · Artifacts
Details
- Import time regressions over 10% are flagged with
⚠️ - Sizes shown as raw / gzip 🗜️. Internal bytes = own code only. Total bytes = with all dependencies. Import time = Node.js cold-start median.
Coverage Delta
Comparing 6 changed files against main @ Overall Coverage
|
binoy14
left a comment
There was a problem hiding this comment.
just one change looks good otherwise
|
|
||
| let workbenchAvailable = false | ||
| try { | ||
| moduleResolve('sanity/workbench', pathToFileURL(path.join(workDir, 'package.json'))) |
There was a problem hiding this comment.
use resolveLocalPackage util instead
c0ad2cf to
f5f0dac
Compare
Co-authored-by: Josh <joshua.ellis18@gmail.com> fix(workbench): allow for a dynamic port (#830)
Co-authored-by: Josh <joshua.ellis18@gmail.com> fix(workbench): allow for a dynamic port (#830)
Coverage Delta
Comparing 6 changed files against main @ Overall Coverage
|
Co-authored-by: Josh <joshua.ellis18@gmail.com> fix(workbench): allow for a dynamic port (#830)
Co-authored-by: Josh <joshua.ellis18@gmail.com> fix(workbench): allow for a dynamic port (#830)
Co-authored-by: Josh <joshua.ellis18@gmail.com> fix(workbench): allow for a dynamic port (#830)
Co-authored-by: Josh <joshua.ellis18@gmail.com> fix(workbench): allow for a dynamic port (#830)
Co-authored-by: Josh <joshua.ellis18@gmail.com> fix(workbench): allow for a dynamic port (#830)
Co-authored-by: Josh <joshua.ellis18@gmail.com> fix(workbench): allow for a dynamic port (#830)
Co-authored-by: Josh <joshua.ellis18@gmail.com> fix(workbench): allow for a dynamic port (#830)
Co-authored-by: Josh <joshua.ellis18@gmail.com> fix(workbench): allow for a dynamic port (#830)
Description
Important
This will go into a feature branch for now and won't be released to
mainanytime soon. It's main purpose is to unblock development ofworkbenchand to showcase how we imagine the setup to work.Setup a workbench dev-server running alongside the studio/ app dev-server. It uses temporary runtime files written to
.sanity/workbenchas Viteroot, similar to how runtime for studios works today.Instead of navigating to the running application directly, users will be prompted to open
workbenchinstead, which is why theportin the config now refers to the workbenchport. This is something that we will likely have to discuss further at some point, because it mixes application and workbench configuration.This needs the following studio PR to work: sanity-io/sanity#12511, which exports
renderWorkbenchfrom@sanity/workbench.Testing
This can be tested using the studio and CLI pre-release together, but it will still fail, because we don't have a remote target yet.
It nevertheless shows that the code that is being run comes from
@sanity/workbenchwhich is what we expect at this point: