🏖️ fix: Sandpack ExternalResources for Static HTML Artifact Previews#12509
Conversation
There was a problem hiding this comment.
Pull request overview
Fixes Sandpack runtime injection errors for static HTML artifact previews by omitting externalResources for the static template, while preserving Tailwind loading for bundled templates and adjusting startup-config query caching behavior.
Changes:
- Scope Sandpack
externalResourcesto non-statictemplates to prevent “Unable to determine file type” injection failures in HTML previews. - Introduce an auth-aware
startupConfigquery key and update allstartupConfigcache reads to use it. - Adjust query-enable timing during login and reduce repeated
getConfigDefaults().interfacecalls by hoisting defaults.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| client/src/components/Artifacts/ArtifactPreview.tsx | Builds Sandpack options per template, omitting externalResources for static previews |
| client/src/utils/artifacts.ts | Provides shared Sandpack options/files (Tailwind CDN + shared /public/index.html) used by artifact previews |
| client/src/data-provider/Endpoints/queries.ts | Adds startupConfigKey(isAuthenticated) and switches useGetStartupConfig to the new key |
| client/src/hooks/SSE/useEventHandlers.ts | Updates startup-config cache reads and consolidates imports via ~/data-provider |
| client/src/hooks/Input/useQueryParams.ts | Updates startup-config cache reads to the auth-aware query key |
| client/src/hooks/Input/useQueryParams.spec.ts | Updates ~/data-provider mocking to preserve real exports like startupConfigKey |
| client/src/hooks/Conversations/useNavigateToConvo.tsx | Updates startup-config cache reads to the auth-aware query key |
| client/src/hooks/Chat/useChatFunctions.ts | Updates startup-config cache reads to the auth-aware query key |
| client/src/components/Nav/SettingsTabs/Data/ImportConversations.tsx | Updates startup-config cache reads to the auth-aware query key and simplifies imports |
| client/src/hooks/Endpoint/useEndpoints.ts | Uses a hoisted default interface config (getConfigDefaults().interface) as fallback |
| client/src/components/Chat/Menus/Endpoints/ModelSelector.tsx | Uses a hoisted default interface config (getConfigDefaults().interface) as fallback |
| client/src/hooks/AuthContext.tsx | Re-enables queries from AuthContext when auth becomes true |
| client/src/data-provider/Auth/mutations.ts | Disables queries during login mutation and re-enables on error (success path now handled by AuthContext) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const options: SandpackProviderProps['options'] = useMemo(() => { | ||
| const baseOptions = template === 'static' ? {} : sharedOptions; | ||
|
|
||
| if (!startupConfig) { | ||
| return sharedOptions; | ||
| return baseOptions; | ||
| } | ||
|
|
||
| return { | ||
| ...sharedOptions, | ||
| ...baseOptions, | ||
| bundlerURL: template === 'static' ? startupConfig.staticBundlerURL : startupConfig.bundlerURL, |
There was a problem hiding this comment.
baseOptions is set to {} for the static template, which drops all entries from sharedOptions (not just externalResources). This works today because sharedOptions only contains externalResources, but it’s easy to introduce a future regression if additional shared Sandpack options are added later and silently stop applying to static previews.
Consider instead preserving sharedOptions while explicitly omitting/emptying externalResources for static (e.g., set externalResources: []), so other shared option defaults remain consistent across templates.
| /** | ||
| * Auth-aware query key so unauthenticated (login page) and authenticated | ||
| * (chat page) configs are cached independently, preventing stale | ||
| * unauthenticated config from persisting after login. | ||
| */ | ||
| export const startupConfigKey = (isAuthenticated: boolean) => | ||
| [QueryKeys.startupConfig, isAuthenticated] as const; | ||
|
|
||
| export const useGetStartupConfig = ( | ||
| config?: UseQueryOptions<t.TStartupConfig>, | ||
| ): QueryObserverResult<t.TStartupConfig> => { | ||
| const queriesEnabled = useRecoilValue<boolean>(store.queriesEnabled); | ||
| const user = useRecoilValue<t.TUser | undefined>(store.user); | ||
| return useQuery<t.TStartupConfig>( | ||
| [QueryKeys.startupConfig], | ||
| startupConfigKey(!!user), | ||
| () => dataService.getStartupConfig(), |
There was a problem hiding this comment.
This PR introduces additional cross-cutting behavior changes beyond the Sandpack externalResources fix: an auth-aware startupConfigKey and updates to the startup config query key shape. This is a non-trivial change that can affect caching/invalidation across the app, but it isn’t mentioned in the PR description.
Please update the PR description to cover this change (or consider splitting it into a separate PR) so reviewers can evaluate the caching/auth implications explicitly.
The Tailwind CDN URL lacks a file extension, causing Sandpack's static template to throw a runtime injection error. Static previews already load Tailwind via a script tag in the shared index.html, so externalResources is unnecessary for them. Closes #12507
danny-avila
force-pushed
the
claude/youthful-murdock
branch
from
4a6cade to
8c7e518
Compare
- Surgically omit only externalResources for static templates instead of discarding all sharedOptions, preventing future regression if new template-agnostic options are added. - Extract options logic into a pure, testable helper function. - Add unit tests covering all template/config combinations.
|
@codex review |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3b7d3346d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Sandpack's static template regex detects resource type from the URL's last file extension. The versioned CDN path (/3.4.17) matched ".17" instead of ".js", throwing "Unable to determine file type". Rather than omitting externalResources for static templates (which would remove the only Tailwind injection path for HTML artifacts that don't embed their own script tag), append a #tailwind.js fragment hint so the regex matches ".js". Fragments are not sent to the server, so the CDN response is unchanged.
|
@codex review |
|
Codex Review: Didn't find any major issues. 👍 ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
danny-avila
changed the title
…anny-avila#12509) * fix: omit externalResources for static Sandpack previews The Tailwind CDN URL lacks a file extension, causing Sandpack's static template to throw a runtime injection error. Static previews already load Tailwind via a script tag in the shared index.html, so externalResources is unnecessary for them. Closes danny-avila#12507 * refactor: extract buildSandpackOptions and add tests - Surgically omit only externalResources for static templates instead of discarding all sharedOptions, preventing future regression if new template-agnostic options are added. - Extract options logic into a pure, testable helper function. - Add unit tests covering all template/config combinations. * chore: fix import order and pin test assertions * fix: use URL fragment hint instead of omitting externalResources Sandpack's static template regex detects resource type from the URL's last file extension. The versioned CDN path (/3.4.17) matched ".17" instead of ".js", throwing "Unable to determine file type". Rather than omitting externalResources for static templates (which would remove the only Tailwind injection path for HTML artifacts that don't embed their own script tag), append a #tailwind.js fragment hint so the regex matches ".js". Fragments are not sent to the server, so the CDN response is unchanged.
…anny-avila#12509) * fix: omit externalResources for static Sandpack previews The Tailwind CDN URL lacks a file extension, causing Sandpack's static template to throw a runtime injection error. Static previews already load Tailwind via a script tag in the shared index.html, so externalResources is unnecessary for them. Closes danny-avila#12507 * refactor: extract buildSandpackOptions and add tests - Surgically omit only externalResources for static templates instead of discarding all sharedOptions, preventing future regression if new template-agnostic options are added. - Extract options logic into a pure, testable helper function. - Add unit tests covering all template/config combinations. * chore: fix import order and pin test assertions * fix: use URL fragment hint instead of omitting externalResources Sandpack's static template regex detects resource type from the URL's last file extension. The versioned CDN path (/3.4.17) matched ".17" instead of ".js", throwing "Unable to determine file type". Rather than omitting externalResources for static templates (which would remove the only Tailwind injection path for HTML artifacts that don't embed their own script tag), append a #tailwind.js fragment hint so the regex matches ".js". Fragments are not sent to the server, so the CDN response is unchanged.
Summary
Fixes #12507
Sandpack's
statictemplate detects external resource type by matching the last file extension in the URL. The versioned Tailwind CDN path (https://cdn.tailwindcss.com/3.4.17) matches.17instead of.js, causinginjectExternalResourcesto throw"Unable to determine file type". Because the throw occurs inside the injection pipeline'stry/catch, it also aborts all subsequent runtime injections (console hook, refresh listener).#tailwind.jsURL fragment hint so Sandpack's regex matches.js. Fragments are not sent to the server, so the CDN response is unchanged.buildSandpackOptionshelper inartifacts.tsfor testability.Change Type
Testing
text/htmlartifact with Tailwind classes (without an explicit Tailwind<script>tag in the content).Runtime injection failederror appears.cd client && npx jest --no-coverage artifacts.test.ts— all tests pass.Checklist