Astro now breaks Sanity integration by misinterpreting config as route -- immediate crash

narration-sd commented 3 months ago

Astro Info

Astro                    v4.5.9
Node                     v20.11.1
System                   Windows (x64)
Package Manager          unknown
Output                   static
Adapter                  none
Integrations             none

If this issue only occurs in one browser, which browser is a problem?

not a browser issue

Describe the Bug

The crash occurs immediately on startup, when a path in @sanity/astro integration config is given a full protocol-domain url signature. A simple slashes path does not cause the breakage.

Looking at the Astro code involved from the stack trace, it seems the experimental code for Containers is being run, though no containers are called for, and that this code may be treating the Sanity config as a route, which it fails on due to the full protocol-domain form. None of this should be involved, and the url is not a route for the application.

Here's the stack trace:

blog:dev: 14:42:20 [types] Generated 4ms
blog:dev: Missing parameter name at 5
blog:dev:   Stack trace:
blog:dev:     at lexer (C:\vger\projects\sani\sapres-astro\node_modules\path-to-regexp\dist\index.js:48:23)
blog:dev:     at compile (C:\vger\projects\sani\sapres-astro\node_modules\path-to-regexp\dist\index.js:181:29)
blog:dev:     at createInjectedRoutes (file:///C:/vger/projects/sani/sapres-astro/node_modules/astro/dist/core/routing/manifest/create.js:213:22)
blog:dev:     at configureServer (file:///C:/vger/projects/sani/sapres-astro/node_modules/astro/dist/vite-plugin-astro-server/plugin.js:28:9)
blog:dev:     at async createContainer (file:///C:/vger/projects/sani/sapres-astro/node_modules/astro/dist/core/dev/container.js:54:22)

Here is the astro.config config segment where the integration's erroring path item is introduced -- as studioBasePath. The similarly configured item stega.studioUrl does not cause the crash, and studioBasePath is also safe if the preview url is commented out of its construction.

PUBLIC_SANITY_STUDIO_PREVIEW_URL is http://localhost:4321, and PUBLIC_SANITY_STUDIO_BASE_PATH: is /admin

export default defineConfig({
  integrations: [
    sanityIntegration({
      projectId: env.PUBLIC_SANITY_PROJECT_ID,
      dataset: env.PUBLIC_SANITY_DATASET,
      apiVersion: env.PUBLIC_SANITY_API_VERSION,
      useCdn: false,
      perspective: 'previewDrafts',
      token: env.PUBLIC_SANITY_VIEWER_TOKEN,
      stega: {
        enabled:true,
        studioUrl: env.PUBLIC_SANITY_STUDIO_PREVIEW_URL
            + env.PUBLIC_SANITY_STUDIO_BASE_PATH,
      },
      studioBasePath: env.PUBLIC_SANITY_STUDIO_PREVIEW_URL + env.PUBLIC_SANITY_STUDIO_BASE_PATH,
    }),
    react(),
  ],
  output: "server",
  adapter: vercel({
    edgeMiddleware: true,
  }),

});

What's the expected result?

The app should simple launch and operate as a simple blog website, as it does when studioBasePathis a simple path and not a full protocol-domain url.

Stackblitz demo is included -- create the .env file with contents in the README to run it,npm run dev and immediately get the crash.

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-gpdyg7?file=README.md

Participation

[ ] I am willing to submit a pull request for this issue.

ascorbic commented 3 months ago

Hi @narration-sd Looking at the @sanity/astro code, it expects studioBasePath to be a relative path. See the code here. That's where it's injecting the route which is causing the problem. Is there a reason you're expecting the full URL to work?

narration-sd commented 3 months ago

Hi @ascorbic , Matt,

Very interesting. Early here, and will be looking into this.

studioBasePath needs to be able to be full domain url whenever the Studio for Sanity would not be served from the same url as the site -- i.e. not embedded. This is actually a pretty common case on production installations.
I'm intrigued that the Sanity developer used this route pathway
at a very quick glance, it seems that modifying the regex that's part of that call might tame things, but this is not assured as there may be an underlying narrowing of the possibility.

I'll come back to you, and quite appreciate the rapid response; wanted here to also respect your time zone, which for some years was once mine :).

ascorbic commented 3 months ago

No problem. I'm going to close this now, because any further discussion would be best taken to the Sanity repo.

withastro / astro