Skip to content

feat(themes): send style.css and script.js through themes:migrate#374

Merged
luis-almeida merged 3 commits into
masterfrom
luis/themes-migrate-css-js
May 29, 2026
Merged

feat(themes): send style.css and script.js through themes:migrate#374
luis-almeida merged 3 commits into
masterfrom
luis/themes-migrate-css-js

Conversation

@luis-almeida
Copy link
Copy Markdown
Contributor

@luis-almeida luis-almeida commented May 28, 2026

Description

zcli themes:migrate previously only sent the theme's .hbs templates to the migration endpoint, ignoring the root style.css and script.js. That blocked any migration that needs to alter raw CSS/JS — e.g. bundling normalize.css into the theme's stylesheet.

This wires both files through: read them from the theme root, send them as css and js keys in the request, then split them out of the response and write the migrated content back to the theme root via a new rewriteJsAndCss helper.

Reads the theme's root style.css and script.js, sends them to the
migration endpoint as `css` and `js`, and writes the migrated content
back to the theme root after splitting them out of the templates
response.
The fs.readFileSync stub matched on a literal forward-slash path, but
migrate.ts uses path.join which produces backslash-separated paths on
Windows. The stub returned undefined there, breaking the request payload
match.
@luis-almeida luis-almeida marked this pull request as ready for review May 28, 2026 13:08
@luis-almeida luis-almeida requested a review from a team as a code owner May 28, 2026 13:08
Copilot AI review requested due to automatic review settings May 28, 2026 13:08
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copy link
Copy Markdown
Contributor

@BrunoBFerreira BrunoBFerreira left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a nitpick.
Looks great 👍

themePath: string,
{ css, js }: { css: string; js: string }
) {
const cssPath = `${themePath}/style.css`
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(nit) Maybe we should use join here to build the string, since I believe it is what we use in other places of the repo.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch! Addressed in 6ac415d

Matches the read side in migrate.ts and addresses a review nit. Tests
follow the same pattern so they stay portable on Windows.
@luis-almeida luis-almeida merged commit ada57bf into master May 29, 2026
7 checks passed
@luis-almeida luis-almeida deleted the luis/themes-migrate-css-js branch May 29, 2026 09:09
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.

3 participants