Conversation
|
The performance metrics CI job fails because it uses the same setup command on |
gcsecsey
left a comment
There was a problem hiding this comment.
Thanks for taking on the task to reorganize to a monorepo @fredrikekelund, I think this is going to make working with the different packages easier on the long run.
Also thanks for the detailed description of the changes, these made reviewing much easier! 🙌
I could npm i from the project root as expected, and using npm ls -ws I can see the newly added packages in the workspace:
When starting the app, I'm currently getting failures to start the site servers, it seems as if the cli package dependencies are not yet installed, or it's not built:
Node.js v22.12.0
[Exit code] 1
at ChildProcess.<anonymous> (/Users/gcsecsey/a8c-projects/studio/.conductor/melbourne/dist/main/index.js:62877:22)
at ChildProcess.emit (node:events:519:28)
at ChildProcess.emit (node:domain:489:12)
at maybeClose (node:internal/child_process:1101:16)
at Socket.<anonymous> (node:internal/child_process:456:11)
at Socket.emit (node:events:519:28)
at Socket.emit (node:domain:489:12)
at Pipe.<anonymous> (node:net:346:12)
[2026-02-12T11:31:54.373Z][info][main] Sentry Logger [log]: Captured error event `[Base message] Failed to start site
[stderr] node:internal/modules/cjs/loader:1252
throw err;
^
Error: Cannot find module '/Users/gcsecsey/a8c-projects/studio/.conductor/melbourne/dist/cli/main.js'
at Function._resolveFilename (node:internal/modules/cjs/loader:1249:15)
at Function._load (node:internal/modules/cjs/loader:1075:27)
at TracingChannel.traceSync (node:diagnostics_channel:322:14)
at wrapModuleLoad (node:internal/modules/cjs/loader:219:24)
at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:170:5)
at node:internal/main/run_main_module:36:49 {
code: 'MODULE_NOT_FOUND',
requireStack: []
}
.gitignore
Outdated
There was a problem hiding this comment.
| apps/studio/e2e/imports/*.tar.gz | |
| apps/studio/e2e/imports/*.zip | |
| apps/studio/e2e/imports/*.wpress |
| /fastlane/README.md | ||
|
|
||
| # CLI npm artifacts | ||
| cli/vendor/ |
There was a problem hiding this comment.
| apps/cli/vendor/ |
There was a problem hiding this comment.
I'm pretty sure that was generated by Playground CLI at some point. Maybe it's not the case anymore
| const packageVersion = JSON.parse( | ||
| readFileSync( resolve( __dirname, 'package.json' ), 'utf-8' ) | ||
| ).version; |
There was a problem hiding this comment.
We didn't add a separate version field to the CLI package.json yet. We could either add one, or point this to the root package.json if we want to have a canonical version.
| const packageVersion = JSON.parse( | |
| readFileSync( resolve( __dirname, 'package.json' ), 'utf-8' ) | |
| ).version; | |
| const packageVersion = JSON.parse( | |
| readFileSync( resolve( __dirname, '../../package.json' ), 'utf-8' ) | |
| ).version; |
There was a problem hiding this comment.
Good point. We used to have a separate version field, but decided to drop it to keep the studio --version command output in sync with the Studio UI.
It makes sense to consolidate this and maintain a single canonical version field. My instinctive preference would be to use the root package.json for this, but if Electron Forge complains, maybe it's better to use apps/studio/package.json. I'll take a look
Hmm, would you mind pulling the latest changes and running |
Thanks for checking @fredrikekelund. After pulling the latest changes, both |
|
The macOS dev build is 601 MB. 1.7.4-beta2 is 579 MB, so we have a 22 MB increase. I don't know exactly what that comes down to, but it's something we can revisit after landing this PR, IMO. |
Related issues
Proposed Changes
Note
The grunt work in this PR was done using OpenAI Codex. 🚨 I know the size of the diff looks daunting, but aside from
package-lock.jsonchanges, this PR is mostly just moving files around, changing import paths and tweaking config files 🚨Note
As noted in #2565 (comment), it's expected that the performance tests are failing.
Warning
A consequence of this change is that
package-lock.jsonwill no longer govern the version of dependencies we ship in production releases. That's because we now have a single lockfile for all npm workspaces, but we use only the package-specificpackage.jsonfiles to install thenode_modulesdirectories shipped in production (without a lockfile). This is probably fine, but it means we should be a bit more conservative about the semver ranges we use inpackage.json. For example, we cannot use^3.0.46for@wp-playgrounddependencies if the intention is to ship3.0.46to production. For those cases, we should save the exact version inpackage.json.The CLI used to be more of an add-on to Studio, but as of v1.7.0, it's become integral to how Studio works. To reflect this, and to make the codebase easier to work with (for humans and AI agents), this PR adopts a proper monorepo structure.
So, what does "proper monorepo structure" mean here?
apps/cliand the Electron app code is moved toapps/studio. They now live side by side.apps/clicode and theapps/studiocode are now isolated – there are no cross imports../commonhad been made into a proper package, using@studio/commonas the package name. It now lives intools/common.@studio/common, so it's not consumed as a true npm module. This is because we'd need to add package exports for everything we want to consume outside that package (i.e., for basically every function/type/class). This seemed tedious to me, but if it's good practice for whatever reason, we can easily revisit it later.tools/, liketools/eslint-plugin-studio.npm installonce in the repo root, and you're good to go.package.jsonfiles and npm reconciles and dedupes everything into a singlenode_modulesdirectory and a singlepackage-lock.jsonfile.node_modulesdirectories that get copied into the packaged output. We handle this withinstall:bundlenpm scripts inapps/cli/package.jsonandapps/studio/package.json.apps/cli/node_modulesandapps/studio/node_modulesdirectories, which potentially messes up the npm workspace-powered dependency tree. So, if you runnpm run packagelocally, you would typically need to rerunnpm ciafter the script finishes to reconcile the dependency tree. It's easy for other developers to be stumped by this requirement, so I addressed it by adding ascripts/package-in-isolation.tsscript. The gist is that it copies the source files from the repo to a temporary directory, installs dependencies, runsinstall:bundleand the package/make script, and then copies the output back to the repo.Testing Instructions
CI should pass, and the dev build should install fine
Pre-merge Checklist