Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feat/studio manifests cont #7403

Merged
merged 63 commits into from
Oct 3, 2024
Merged

Feat/studio manifests cont #7403

merged 63 commits into from
Oct 3, 2024

Conversation

snorrees
Copy link
Contributor

@snorrees snorrees commented Aug 21, 2024

Studio manifest for Create

The Create project needs access to workspaces and schema define in the Studio.

Create will use the schema to allow users to automatically map their documents into a Studio document type.
Create needs workspace information to deep-link into studios.

This PR introduces "manifest extract", which makes workspace and schema details HTTP accessible for sanity deploy'ed studios.

Timeline

Create is fine with using a tagged release of studio until this PR gets merged and released.

PR includes

  • Everything and the kitchen-sink manifest format tailored for Create:
    • For every workspace, we include most everything that is serializable on the schema; options, unknown properties, the works.
    • Validation is serialized to a best-effort format, including any serializable data from the RuleSpec.
    • This allows Create to specify a schema-driven API for configuring Create-Studio integration, as needed, without needing changes to core.
  • New command sanity manifest extract.
    • No external users should rely on the output files. They are for internal use only.
  • Runs manifest extract during sanity deploy commands
    • This becomes default behavior, and there is no opt out.
    • When manifest extract fails, we show a soft warning; it should never result in a failed deploy.
    • Note, we do NOT run manifest extract for sanity build
    • 2 minute timeout

The following files are produced

  • create-manifest.json
  • <schema-hash>.create-schema.json

By default these gets written to /dist/static, and will therefor be served from <studio-url>/static/<filename> when using sanity deploy.

Custom deploys will be expected to serve the files under <studio-url>/static/ as well.
For Next.js, this can be done by using sanity manifest extract --path /public/static && next build, and putting the generated files on .gitignore.

History

@juice49 graciously provided most of the implementation; I have adjusted it further as the requirements has changed.

Failure states

When running sanity extract manifest we do want to return an error code and show the exception, since it is explicit intent, and you want a manifest (and should be able to know why it failed).

Example when ran in test-studio:

> sanity manifest extract                                        
ℹ️ Unable to extract manifest. Certain features like Sanity Create will not work with this studio.
For more information, see: https://www.sanity.io/docs/cli

TypeError: Failed to load configuration file "<path>/dev/test-studio/sanity.config.ts":
groq__default.default is not a function

When running sanity deploy, we dont want to return an error code, or fail the deploy:

Example when ran in test-studio (with custom studioHost):

> sanity deploy    
✅ Checking project info
Your project has not been assigned a studio hostname.
Creating https://sanity-manifest-test.sanity.studio

✅ Creating studio hostname
✅ Clean output folder (37ms)
✅ Build Sanity Studio (33941ms)
ℹ️ Unable to extract manifest. Certain features like Sanity Create will not work with this studio.
For more information, see: https://www.sanity.io/docs/cli
✅ Verifying local content
✅ Deploying to Sanity.Studio

Manifest extract will not stop deployment, and is also does not show a big fat ❌ (could be scary to devs, since this is now default)

Create - Studio integration

  • Upgrade your studio to the new version
  • Deploy
  • The studio (with schema) can now be selected in Create

For a broader discussion internally, refer to this SCQA

What to review

  • Are we ok with the proposed command?
  • Is it ok that we dont include a --format argument at this time?
  • Think about failure cases with sanity deploy. Are there ways in which manifest extract could prevent sanity deploy from working?
  • Do we need to worry about hypothetical timeouts with the extract task? (We set a 2 minute cap now)

Headsup: some of the files are kinda big; remember to expand them in the diff when reviewing.

Testing

Consider the jest tests: are they exhaustive enough?

Manual testing

In a project you can deploy using sanity deploy:

  • npm i sanity@manifests
  • npm run deploy (or npm run build && npm run start to run on localhost)
  • Visit <studioHost>/static/create-manifest.json
  • Visit a workspace schema (<studioHost>/static/<schema-path-found-in-workspace> on the form <schema-hash>.create-schema.json)

Check the SCQA doc for how to test with Create

Testing commands:

  • sanity should include manifest command group
  • sanity manifest should list sub commands (currently only extract)
  • sanity manifest extract --help should show help for the new command
  • sanity manifest extract in a project should extract the manifest (or fail gracefully)

Regression testing

  • sanity schema extract should work as before
    (this branch should not make any changes to the codepath, this is just here to sanity check that reverted changes didnt make it back in)

Known issues

  • test-studio has "too complex" schema; manifest extract as it does not work for these kinds of studios.
    • manifest extract has the same limitations as graphql deploy; the schema has to be parsable in the mock-browser env we are using
  • the extracted manifest does not extract structure paths (possibly needed to create deep-links for certain configurations)
  • the extracted manifest not extract auth provider (possibly needed to correctly show auth screen for studio login inside Create)

Notes for release

No need to mention this feature yet.

Copy link

vercel bot commented Aug 21, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
page-building-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 3, 2024 8:45am
performance-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 3, 2024 8:45am
test-compiled-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 3, 2024 8:45am
test-next-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 3, 2024 8:45am
test-studio ✅ Ready (Inspect) Visit Preview 💬 Add feedback Oct 3, 2024 8:45am
1 Skipped Deployment
Name Status Preview Comments Updated (UTC)
studio-workshop ⬜️ Ignored (Inspect) Visit Preview Oct 3, 2024 8:45am

@snorrees
Copy link
Contributor Author

snorrees commented Oct 1, 2024

All work on this leg of the Create integration is now concluded as far as I see it.

I've re-tested the latest commit with a tagged release (3.57.2-manifests.74) on another project.

Compatible schema

> sanity manifest extract
✅ Extracted manifest (1578ms)
> sanity deploy
✅ Checking project info
✅ Clean output folder (19ms)
✅ Build Sanity Studio (10714ms)
✅ Extracted manifest (1681ms)
✅ Verifying local content
✅ Deploying to Sanity.Studio

Success! Studio deployed to <redacted>

Incompatible schema

> sanity manifest extract
ℹ️ Couldn't extract manifest file. Sanity Create will not be available for the studio.
Disable this message with SANITY_CLI_EXTRACT_MANIFEST_ENABLED=false

Error: Failed to load configuration file "sanity.config.ts":
hardcoded-error
(...stacktrace)
> sanity deploy
✅ Checking project info
✅ Clean output folder (19ms)
✅ Build Sanity Studio (10714ms)
ℹ️ Couldn't extract manifest file. Sanity Create will not be available for the studio.
Disable this message with SANITY_CLI_EXTRACT_MANIFEST_ENABLED=false
✅ Verifying local content
✅ Deploying to Sanity.Studio

Success! Studio deployed to <redacted>
SANITY_CLI_EXTRACT_MANIFEST_ENABLED=false npm run deploy

> sanity deploy

✅ Checking project info
✅ Clean output folder (16ms)
✅ Build Sanity Studio (10313ms)
✅ Verifying local content
✅ Deploying to Sanity.Studio

Success! Studio deployed to <redacted>

Bonus win, running localhost with manifest exposed can be done with

npx sanity manifest extract --path public/static && npm run dev

(no hot reload, but thats ok)

@snorrees snorrees requested a review from ricokahler October 1, 2024 19:56
@snorrees
Copy link
Contributor Author

snorrees commented Oct 2, 2024

ping @ricokahler @ryanbonial great if ya'll could look over my latest comments and changes, and review.

The idea is to get this into next weeks release, so we can shore up this part of the Create integration.
(more chapters to come in other PRs)

Copy link
Contributor

github-actions bot commented Oct 2, 2024

⚡️ Editor Performance Report

Updated Thu, 03 Oct 2024 08:54:41 GMT

Benchmark reference
latency of sanity@latest
experiment
latency of this branch
Δ (%)
latency difference
article (title) 18.2 efps (55ms) 16.7 efps (60ms) +5ms (+9.1%)
article (body) 53.8 efps (19ms) 56.5 efps (18ms) -1ms (-4.8%)
article (string inside object) 19.6 efps (51ms) 18.0 efps (56ms) +5ms (+8.8%)
article (string inside array) 14.7 efps (68ms) 13.9 efps (72ms) +4ms (+5.9%)
recipe (name) 30.3 efps (33ms) 30.3 efps (33ms) +0ms (-/-%)
recipe (description) 34.5 efps (29ms) 33.9 efps (30ms) +1ms (+1.7%)
recipe (instructions) 99.9+ efps (7ms) 99.9+ efps (6ms) -0ms (-/-%)
synthetic (title) 15.0 efps (67ms) 14.9 efps (67ms) +1ms (+0.8%)
synthetic (string inside object) 16.1 efps (62ms) 15.2 efps (66ms) +4ms (+6.5%)

efps — editor "frames per second". The number of updates assumed to be possible within a second.

Derived from input latency. efps = 1000 / input_latency

Detailed information

🏠 Reference result

The performance result of sanity@latest

Benchmark latency p75 p90 p99 blocking time test duration
article (title) 55ms 61ms 73ms 179ms 1134ms 15.8s
article (body) 19ms 21ms 51ms 160ms 265ms 6.0s
article (string inside object) 51ms 54ms 58ms 112ms 761ms 8.5s
article (string inside array) 68ms 71ms 80ms 173ms 1706ms 10.1s
recipe (name) 33ms 36ms 42ms 65ms 44ms 9.7s
recipe (description) 29ms 32ms 36ms 59ms 52ms 6.3s
recipe (instructions) 7ms 8ms 9ms 22ms 0ms 3.3s
synthetic (title) 67ms 72ms 88ms 292ms 2132ms 17.4s
synthetic (string inside object) 62ms 65ms 70ms 247ms 1708ms 10.4s

🧪 Experiment result

The performance result of this branch

Benchmark latency p75 p90 p99 blocking time test duration
article (title) 60ms 66ms 77ms 224ms 1399ms 15.2s
article (body) 18ms 19ms 22ms 139ms 206ms 5.6s
article (string inside object) 56ms 59ms 67ms 150ms 1108ms 9.0s
article (string inside array) 72ms 75ms 83ms 145ms 2013ms 10.2s
recipe (name) 33ms 37ms 71ms 77ms 63ms 9.8s
recipe (description) 30ms 32ms 36ms 58ms 23ms 6.2s
recipe (instructions) 6ms 8ms 10ms 10ms 0ms 3.3s
synthetic (title) 67ms 69ms 74ms 229ms 2098ms 17.0s
synthetic (string inside object) 66ms 69ms 75ms 260ms 1976ms 10.6s

📚 Glossary

column definitions

  • benchmark — the name of the test, e.g. "article", followed by the label of the field being measured, e.g. "(title)".
  • latency — the time between when a key was pressed and when it was rendered. derived from a set of samples. the median (p50) is shown to show the most common latency.
  • p75 — the 75th percentile of the input latency in the test run. 75% of the sampled inputs in this benchmark were processed faster than this value. this provides insight into the upper range of typical performance.
  • p90 — the 90th percentile of the input latency in the test run. 90% of the sampled inputs were faster than this. this metric helps identify slower interactions that occurred less frequently during the benchmark.
  • p99 — the 99th percentile of the input latency in the test run. only 1% of sampled inputs were slower than this. this represents the worst-case scenarios encountered during the benchmark, useful for identifying potential performance outliers.
  • blocking time — the total time during which the main thread was blocked, preventing user input and UI updates. this metric helps identify performance bottlenecks that may cause the interface to feel unresponsive.
  • test duration — how long the test run took to complete.

@snorrees snorrees added this pull request to the merge queue Oct 3, 2024
Merged via the queue into next with commit 1f57db4 Oct 3, 2024
34 of 39 checks passed
@snorrees snorrees deleted the feat/studio-manifests-cont branch October 3, 2024 08:34
bjoerge pushed a commit that referenced this pull request Oct 3, 2024
* feat(sanity): allow `extractSchema` worker to emit schemas for all workspaces

* feat(sanity): include workspace and dataset names when extracting schema

* feat(cli): add `manifest` commands

* feat(manifest): add `@sanity/manifest` package

* refactor(sanity): use manifest schemas from `@sanity/manifest`

* chore: format files

* feat(schema): include `title`, `description`, and `deprecated` attributes when extracting schema

* feat(sanity): add `direct` schema format to schema extractor

* Revert "feat(schema): include `title`, `description`, and `deprecated` attributes when extracting schema"

This reverts commit 60cb576.

* feat(sanity): export `ConcreteRuleClass` class

* feat(sanity): include validation rules in manifests

* refactor(sanity): move manifest extraction code

* feat(sanity): extract manifest during build

* feat(sanity): adopt `.studioschema.json` filename suffix for manifest schemas

* refactor(sanity): rename manifest extraction functions (remove plural)

* fix(sanity): remove redundant success message

* fix(sanity): stop build spinner before starting manifest extraction

* feat(sanity): add `unstable_extractManifestOnBuild` CLI config option

* feat(test-studio): enable `unstable_extractManifestOnBuild`

* fix(sanity): switch to node crypto for node 18 compatibility

* feat(cli): add `unstable_staticAssetsPath` CLI configuration option

* chore(cli): refine `unstable_extractManifestOnBuild` CLI configuration option description

* feat(sanity): remove extraneous `types` wrapper from manifests

* debug(test-studio): remove Mux plugin to unblock typegen

* feat(embedded-studio): enable manifest extraction

* feat(starter-next-studio): enable manifest extraction

* wip

* feat(sanity): normalize type constraints in manifest validation

* wip

* chore: merge fix

* feat: serialize userland properties and validation rules in manifest

* fix: remove @sanity/manifest package

* chore: cleanup

* fix: serialize fieldsets

* fix: omit default titles on fields and array-members

* fix: ensure manifest schema is restoreble and supports cross dataset references

* chore: mergefix

* fix: serialization of type aliases no longer inlines fields and of props

* fix: removes double dot in filename

* feat: manifest command

* chore: tweaks

* chore: revert redundant changes

* fix: adds manifest group to CLI

* chore: wording change

* fix: adds a 2-minute timeout to manifest extract

* fix: ensures error code when mainfest extract fails and changes failed spinner message to info

* chore: use *ENABLED instead of *DISABLED for constant

* chore: defensive optional chaining for option extraction

* chore: reworded EXTRACT_FAILURE_MESSAGE

---------

Co-authored-by: Ash <[email protected]>
ricokahler pushed a commit that referenced this pull request Oct 4, 2024
* feat(sanity): allow `extractSchema` worker to emit schemas for all workspaces

* feat(sanity): include workspace and dataset names when extracting schema

* feat(cli): add `manifest` commands

* feat(manifest): add `@sanity/manifest` package

* refactor(sanity): use manifest schemas from `@sanity/manifest`

* chore: format files

* feat(schema): include `title`, `description`, and `deprecated` attributes when extracting schema

* feat(sanity): add `direct` schema format to schema extractor

* Revert "feat(schema): include `title`, `description`, and `deprecated` attributes when extracting schema"

This reverts commit 60cb576.

* feat(sanity): export `ConcreteRuleClass` class

* feat(sanity): include validation rules in manifests

* refactor(sanity): move manifest extraction code

* feat(sanity): extract manifest during build

* feat(sanity): adopt `.studioschema.json` filename suffix for manifest schemas

* refactor(sanity): rename manifest extraction functions (remove plural)

* fix(sanity): remove redundant success message

* fix(sanity): stop build spinner before starting manifest extraction

* feat(sanity): add `unstable_extractManifestOnBuild` CLI config option

* feat(test-studio): enable `unstable_extractManifestOnBuild`

* fix(sanity): switch to node crypto for node 18 compatibility

* feat(cli): add `unstable_staticAssetsPath` CLI configuration option

* chore(cli): refine `unstable_extractManifestOnBuild` CLI configuration option description

* feat(sanity): remove extraneous `types` wrapper from manifests

* debug(test-studio): remove Mux plugin to unblock typegen

* feat(embedded-studio): enable manifest extraction

* feat(starter-next-studio): enable manifest extraction

* wip

* feat(sanity): normalize type constraints in manifest validation

* wip

* chore: merge fix

* feat: serialize userland properties and validation rules in manifest

* fix: remove @sanity/manifest package

* chore: cleanup

* fix: serialize fieldsets

* fix: omit default titles on fields and array-members

* fix: ensure manifest schema is restoreble and supports cross dataset references

* chore: mergefix

* fix: serialization of type aliases no longer inlines fields and of props

* fix: removes double dot in filename

* feat: manifest command

* chore: tweaks

* chore: revert redundant changes

* fix: adds manifest group to CLI

* chore: wording change

* fix: adds a 2-minute timeout to manifest extract

* fix: ensures error code when mainfest extract fails and changes failed spinner message to info

* chore: use *ENABLED instead of *DISABLED for constant

* chore: defensive optional chaining for option extraction

* chore: reworded EXTRACT_FAILURE_MESSAGE

---------

Co-authored-by: Ash <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants