feat(core): add userStyle option to ensure highest priority, closes #1110#1695
feat(core): add userStyle option to ensure highest priority, closes #1110#1695Mister-Hope wants to merge 1 commit intomainfrom
Conversation
Pull Request Test Coverage Report for Build 23324886226Details
💛 - Coveralls |
There was a problem hiding this comment.
Pull request overview
Adds a userStyle theme option intended to inject a user-provided style file late in the client entry, so it has higher CSS precedence in the final bundle.
Changes:
- Extend core types (
ThemeObject,ThemeInfo,App) with an optionaluserStylestring. - Resolve
userStylethrough theme inheritance and attach it to the app instance. - Inject
userStyleas an additional import in the generated@internal/clientConfigstemp module.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/core/src/types/theme.ts | Adds userStyle?: string to theme typing and resolved theme info typing. |
| packages/core/src/types/app/app.ts | Adds userStyle?: string onto the App type surface. |
| packages/core/src/app/setupAppThemeAndPlugins.ts | Copies resolved themeInfo.userStyle onto app.userStyle. |
| packages/core/src/app/resolveThemeInfo.ts | Attempts to merge userStyle through theme inheritance. |
| packages/core/src/app/prepare/prepareClientConfigs.ts | Adds an import for app.userStyle in the generated client configs entry. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| themeObject.templateBuildRenderer ?? | ||
| parentThemeInfo.templateBuildRenderer, | ||
| templateDev: themeObject.templateDev ?? parentThemeInfo.templateDev, | ||
| userStyle: themeObject.userStyle ?? parentThemeInfo.userStyle, | ||
| } |
There was a problem hiding this comment.
resolveThemeInfo() never includes userStyle in the base themeInfo object, so themes that do not extend a parent theme will always end up with app.userStyle as undefined even if themeObject.userStyle is set. Include userStyle: themeObject.userStyle in the initial themeInfo (and/or ensure the non-extends branch returns it).
| templateDev: themeObject.templateDev ?? parentThemeInfo.templateDev, | ||
| userStyle: themeObject.userStyle ?? parentThemeInfo.userStyle, | ||
| } |
There was a problem hiding this comment.
The new userStyle resolution logic should be covered by unit tests (there are already tests for resolveThemeInfo). Add cases asserting userStyle is returned for a theme with no parent, and that child themes correctly override / inherit userStyle from parents.
| (filePath, index) => `import * as clientConfig${index} from '${filePath}'`, | ||
| ) | ||
| .join('\n')} | ||
| ${app.userStyle ? `\nimport '${app.userStyle}'` : ''} |
There was a problem hiding this comment.
app.userStyle is interpolated directly into a JS import string literal. If the path contains backslashes (Windows), quotes, or other escapable characters, it can produce invalid generated code. Prefer using JSON.stringify(app.userStyle) (and ideally the same for clientConfigFiles) when generating import specifiers so they are safely escaped.
| (filePath, index) => `import * as clientConfig${index} from '${filePath}'`, | |
| ) | |
| .join('\n')} | |
| ${app.userStyle ? `\nimport '${app.userStyle}'` : ''} | |
| (filePath, index) => `import * as clientConfig${index} from ${JSON.stringify(filePath)}`, | |
| ) | |
| .join('\n')} | |
| ${app.userStyle ? `\nimport ${JSON.stringify(app.userStyle)}` : ''} |
| /** | ||
| * Allow specifying user styles, which will be injected into client at the bottom | ||
| */ | ||
| userStyle?: string |
There was a problem hiding this comment.
userStyle is assigned during app creation (setupAppThemeAndPlugins() is called from createDevApp/createBuildApp before app.init()), so placing it under AppPropertiesInitialized and documenting it as only available after initialization is misleading. Consider moving userStyle to AppPropertiesBase, or adjust the initialization flow/docs so it truly becomes available only after init().
No description provided.