diff --git a/docs/cli/architecture.md b/docs/cli/architecture.md index 0ebb02caed4..4bb5354f73d 100644 --- a/docs/cli/architecture.md +++ b/docs/cli/architecture.md @@ -4,8 +4,8 @@ The Shopify CLI is written in [Typescript](https://www.typescriptlang.org/) and We settled on a **Javascript-based** stack because it aligns with the programming language used to build for the platform, and allows having a strongly-typed contract with web tooling that's necessary to compile and optimize projects for deployment. It's designed following a **modular** architecture where the bottom-most layer represents the foundation that all the features build upon, -and the top-most layer represents an horizontally-distributed set of features that users can opt into based on their needs. -Modularization also encourages clearly defined boundaries that leads to a better structure that's easier to maintain long-term. +and the top-most layer represents a horizontally-distributed set of features that users can opt into based on their needs. +Modularization also encourages clearly defined boundaries that lead to a better structure that's easier to maintain long-term. ## Modules (packages) diff --git a/docs/cli/conventions.md b/docs/cli/conventions.md index fd17816ff49..6078c04d564 100644 --- a/docs/cli/conventions.md +++ b/docs/cli/conventions.md @@ -131,7 +131,7 @@ export default class Dev extends Command { ##### Naming and directory conventions -Services live under the `services` directory inside `cli`. For services that represent a command's business logic, the name must match the name of the command represent. +Services live under the `services` directory inside `cli`. For services that represent a command's business logic, the name must match the name of the command it represents. ``` app/ @@ -174,7 +174,7 @@ app/ Some additional patterns have emerged over time and that don't fit any of the MCS groups: - **Prompts:** Prompts are a particular type of service that prompts the user, collects, and returns the responses as a Javascript object. We recommend creating them under the `prompts/` directory. -- **Utilities:** Utilities represent a sub-domain of responsibilities. For example, we can have a `server` utility that spins up an server to handle HTTP requests. +- **Utilities:** Utilities represent a sub-domain of responsibilities. For example, we can have a `server` utility that spins up a server to handle HTTP requests. ## 3 - @shopify/cli-kit @@ -183,7 +183,7 @@ Some additional patterns have emerged over time and that don't fit any of the MC **Public** modules must live in the `src/public` directory in any of the following sub-directories: - `node`: For modules that are dependent on the Node runtime. -- `browser` For the modules that are dependent on the browser runtime. +- `browser`: For modules that are dependent on the browser runtime. - `common`: For modules that are runtime-agnostic. The sub-organization helps clarify the runtime the functions exported by the module can run. For example, if we provide utilities for manipulating arrays, we'd create them in a `src/public/common/array.ts` module, and the consumers of the `@shopify/cli-kit` package would import them as: diff --git a/docs/cli/cross-os-compatibility.md b/docs/cli/cross-os-compatibility.md index 9d2643a2799..7c98693fa96 100644 --- a/docs/cli/cross-os-compatibility.md +++ b/docs/cli/cross-os-compatibility.md @@ -7,7 +7,7 @@ Supporting the three OSs has implications in how the code is written and tested. ### Use `@shopify/cli-kit` modules -Unlike programming languages like Rust or Go, whose standard library work more consistently across OS, that's not the case with the Node runtime in which the CLI runs. Consequently, packages like [cross-zip](https://www.npmjs.com/package/cross-zip), [execa](https://www.npmjs.com/package/execa), or [pathe](https://www.npmjs.com/package/pathe) in the NPM ecosystem provide a cross-OS-compatible version of the Node APIs. `@shopify/cli-kit` exports modules like system or file that abstract away the usage of those packages, and **thus CLI features must use those modules over the ones provided by Node**. Using `@shopify/cli-kit` modules also eases rolling out fixes, improvements, and optimizations because all features go through the same set of APIs we control. +Unlike programming languages like Rust or Go, whose standard library works more consistently across OS, that's not the case with the Node runtime in which the CLI runs. Consequently, packages like [cross-zip](https://www.npmjs.com/package/cross-zip), [execa](https://www.npmjs.com/package/execa), or [pathe](https://www.npmjs.com/package/pathe) in the NPM ecosystem provide a cross-OS-compatible version of the Node APIs. `@shopify/cli-kit` exports modules like system or file that abstract away the usage of those packages, and **thus CLI features must use those modules over the ones provided by Node**. Using `@shopify/cli-kit` modules also eases rolling out fixes, improvements, and optimizations because all features go through the same set of APIs we control. ## Testing @@ -25,7 +25,7 @@ Please don't assume that a successful working workflow in the OS in which it was After installing Ubuntu 22 then run: - `sudo apt-get update && sudo apt-get -y upgrade` -- `curl -fsSL https://deb.nodesource.com/setup_18.x | sudo -E bash -` +- `curl -fsSL https://deb.nodesource.com/setup_24.x | sudo -E bash -` - `sudo apt-get install -y git nodejs` - `curl -fsSL https://get.pnpm.io/install.sh | sh -` diff --git a/docs/cli/get-started.md b/docs/cli/get-started.md index 259eabda141..06cb6d8391d 100644 --- a/docs/cli/get-started.md +++ b/docs/cli/get-started.md @@ -65,12 +65,12 @@ You can also pass these optional flags: ### More automation -Besides the scripts for building and running the CLIs, there are others that might come handy when adding code to the project: +Besides the scripts for building and running the CLIs, there are others that might come in handy when adding code to the project: - `pnpm test`: Runs the tests of all the packages. - `pnpm lint`: Runs ESLint and Prettier checks for all the packages. - `pnpm lint:fix`: Runs ESLint and Prettier checks for all the packages and fixes the fixable issues. -- `pnpm type-check`: Type-checks all the packagesusing the Typescript `tsc` tool. +- `pnpm type-check`: Type-checks all the packages using the TypeScript `tsc` tool. - `pnpm clean`: Removes the `dist` directory from all the packages. All the packages in the repository contain the above scripts so they can be executed too for an individual package. diff --git a/docs/cli/performance.md b/docs/cli/performance.md index 0af0c53b308..efbbc44e981 100644 --- a/docs/cli/performance.md +++ b/docs/cli/performance.md @@ -31,12 +31,12 @@ We **strongly recommend** reading [this series of blog posts](https://marvinh.de ### Dependencies will most likely have a cost -When NPM dependencies are used in SPAs, they are tree-shaken through bundling tools like ESBuild, Webpack, or Rollup. Because of it, many of them are designed with the implicit assumption that they'll be tree-shaken and exported as a single module (e.g., index.js) that loads the entire graph, **including the modules you are not using**. We could have a similar tooling in the CLI project, but we decided to keep the tooling stack as lean as possible and thus prevent issues that might arise due to the tooling indirection (e.g., invalid source maps or code that don't map 1-to-1 to the source and complicates debugging). Therefore we recommend that: +When NPM dependencies are used in SPAs, they are tree-shaken through bundling tools like ESBuild, Webpack, or Rollup. Because of it, many of them are designed with the implicit assumption that they'll be tree-shaken and exported as a single module (e.g., index.js) that loads the entire graph, **including the modules you are not using**. We could have a similar tooling in the CLI project, but we decided to keep the tooling stack as lean as possible and thus prevent issues that might arise due to the tooling indirection (e.g., invalid source maps or code that doesn't map 1-to-1 to the source and complicates debugging). Therefore we recommend that: - You avoid dependencies unless they are strictly necessary. Bring it up to the team in case of doubt. - When deciding on a dependency, their interface must be modular (many exports over a single one). In other words, avoid monolithic dependencies. - If the dependency is large and uses ESM, use dynamic imports to import it. Note that it'll make the dependent modules' APIs asynchronous, but it'll be improved once this [TC39 proposal](https://github.com/tc39/proposal-defer-import-eval) lands. -- As a **last resource**, if a dependency is a bottleneck, you can use its CJS version or dynamically import it when needed using `await import("my-dependency")`. +- As a **last resort**, if a dependency is a bottleneck, you can use its CJS version or dynamically import it when needed using `await import("my-dependency")`. ### Use concurrency whenever possible diff --git a/docs/cli/testing-strategy.md b/docs/cli/testing-strategy.md index 6244f7a2a9a..29def95288e 100644 --- a/docs/cli/testing-strategy.md +++ b/docs/cli/testing-strategy.md @@ -26,7 +26,7 @@ test("loads the app", async () => { const got = await load() // Then - expect(app.name).toEqual("my-app") + expect(got.name).toEqual("my-app") }) ``` @@ -59,7 +59,7 @@ test("writes", async () => { > :bulb: **Given/When/Then** > -> We recommend grouping the test steps following [Gherkin](https://cucumber.io/docs/gherkin/reference/)'s blocks, given, when, and using code comments. That makes the code test easier to parse visually. +> We recommend grouping the test steps following [Gherkin](https://cucumber.io/docs/gherkin/reference/)'s blocks — given, when, and then — using code comments. That makes the test code easier to parse visually. > :exclamation: **Tests and promises** > @@ -77,8 +77,8 @@ test("writes", async () => { End-to-end tests live under `packages/e2e` and are implemented using [Playwright](https://playwright.dev/). They test full user journeys by invoking the CLI and verifying outputs. Run them with `pnpm test:e2e`. -## Github Actions -Before being able to marge a PR, it must pass all CI checks executed in Github Actions. +## GitHub Actions +Before being able to merge a PR, it must pass all CI checks executed in GitHub Actions. The jobs will detect what packages have changed in that PR and execute the tests only for those. If you want to execute all the tests for all the packages you can manually schedule a workflow through `Actions -> shopify-cli -> Run workflow` diff --git a/docs/cli/troubleshooting.md b/docs/cli/troubleshooting.md index a83d32cd8ca..15a269cc5b3 100644 --- a/docs/cli/troubleshooting.md +++ b/docs/cli/troubleshooting.md @@ -37,7 +37,7 @@ But it doesn't work for us, looks like there is some bug that won't let us mock The correct (and cleaner!) approach would be: ```typescript -const import {functionA} from 'my-module' +import {functionA} from 'my-module' // This will mock all functions with vi.fn() by default vi.mock('my-module')