Add params: startScript, zenMode, sidebarView #4
Conversation
fvsch
changed the title
fvsch
force-pushed
the
fvsch/add-zenmode-param
branch
from
b240208 to
814d8ce
Compare
fvsch
force-pushed
the
fvsch/add-zenmode-param
branch
from
814d8ce to
9511de3
Compare
| /** | ||
| * Supported editor themes | ||
| */ | ||
| export const UI_THEMES = ['light', 'dark'] as const; |
There was a problem hiding this comment.
Defined available themes and UI views as arrays of strings, which we can use in two ways:
- Derive the TypeScript union type for the
themeandviewoptions from those arrays (insrc/interfaces.ts). - Use those arrays when validating user-provided values at runtime (in
src/params.ts)
Before that, we had separate sources of truth.
| * By default, the Console will appear collapsed, and can be opened by users. | ||
| * This option is ignored in WebContainers-based projects. | ||
| */ | ||
| devToolsHeight?: number; |
There was a problem hiding this comment.
There's a bit of noise in this PR because I alphabetized the keys in this interface.
It might be easier to open this file as a standalone file, and review the types and descriptions for startScript, sidebarView and zenMode that way.
| * Defaults to looking for a `dev` script or a `start` script in WebContainers-based project. Ignored in EngineBlock projects. | ||
| */ | ||
| showSidebar?: boolean; | ||
| startScript?: string; |
There was a problem hiding this comment.
Technically we support multiple startScript parameters (up to 3), so the correct type here would be string | [string] | [string, string] | [string, string, string].
But our support for multiple startScript parameters was driven by a rare need, and is partly broken: we open several terminals, and run in parallel, but don't wait for the installation of dependencies to be done before running those scripts.
So if you specify startScript=build&startScript=serve, we will have two Terminals running this in parallel:
npm install && npm run buildnpm run serve
And if the serve script relies on any dependency, or on the build script being done, it will fail.
We do support startScript=build,serve which will run npm install && npm run build && npm run serve sequentially.
Given that, I think it's a good move to restrict the SDK to only emit a single startScript query parameter.
| showSidebar: (value: ParamOptions['showSidebar']) => booleanParam('showSidebar', value), | ||
| sidebarView: (value: ParamOptions['sidebarView']) => | ||
| enumParam('sidebarView', value, UI_SIDEBAR_VIEWS), | ||
| startScript: (value: ParamOptions['startScript']) => stringParams('startScript', value), |
There was a problem hiding this comment.
The actual implementation that accepts the new options and produces query parameters is happening here.
Most of the changes in this PR are types, descriptions, and updates to unit tests. The implementation itself is just allowing values to be passed along from an options object into a query string, with simple validation & encoding.
| if (typeof value === 'number' && !Number.isNaN(value)) { | ||
| const clamped = Math.min(100, Math.max(0, value)); | ||
| return `${name}=${Math.round(clamped)}`; | ||
| return `${name}=${encodeURIComponent(Math.round(clamped))}`; |
There was a problem hiding this comment.
I don’t think Math.round can produce an output that contains problematic characters when stringified, but it doesn't cost much to call encodeURIComponent to encode the value anyway.
| allowList: readonly string[] = [] | ||
| ): string { | ||
| if (allowList.includes(value)) { | ||
| return `${name}=${encodeURIComponent(value)}`; |
There was a problem hiding this comment.
So far the lists of enum values we use don't contain characters that need encoding, but it doesn't cost much to encode values with encodeURIComponent just in case.
| return values | ||
| .filter((val) => typeof val === 'string' && val.trim() !== '') | ||
| .map((val) => `${name}=${encodeURIComponent(val!.trim())}`); | ||
| .map((val) => `${name}=${encodeURIComponent(val!)}`) |
There was a problem hiding this comment.
We stop trimming user-provided strings. That means that if a user provides a file path like ' src/components/App.js' but the ' src' folder doesn't exist (only the 'src' folder), things might break.
On the other hand, if they're trying to use a npm script name that had whitespace for some reason, e.g. "serve ", then this makes it possible.
Better to have less hidden magic that our users cannot work around, IMHO.
sylwiavargas
left a comment
sylwiavargas
left a comment
There was a problem hiding this comment.
thank you for this PR and your thorough walkthrough 🙏
This PR adds support for three options in
EmbedOptionsandOpenOptions:startScript(WebContainers projects only): specify the npm script to run on project loadzenMode(inOpenOptionsonly): turns on zen mode for the classic editorsidebarView: select which sidebar view to show initiallyThose query parameters are already supported on stackblitz.com. They may or may not be supported on StackBlitz EE depending on the deployed version; when not supported, they would be ignored.