Skip to content

feat: recursively copy cwd into temp folder#2479

Merged
matthewvolk merged 1 commit into
canaryfrom
CATALYST-1386-copy-to-temp
Jul 16, 2025
Merged

feat: recursively copy cwd into temp folder#2479
matthewvolk merged 1 commit into
canaryfrom
CATALYST-1386-copy-to-temp

Conversation

@matthewvolk

Copy link
Copy Markdown
Contributor

What/Why?

  • Adds consola for logging
  • Runs CLI tests in all popular OS's to match the different environments where our customers run the CLI
  • Recursively copy a Catalyst project to a temporary directory, OS-agnostic API's from Node.js are being used
  • Includes tests

Testing

Includes unit tests

Migration

N/A

@matthewvolk
matthewvolk requested a review from a team July 16, 2025 15:29
@changeset-bot

changeset-bot Bot commented Jul 16, 2025

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: 91d0d22

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel

vercel Bot commented Jul 16, 2025

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
catalyst-canary ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jul 16, 2025 6:09pm
4 Skipped Deployments
Name Status Preview Comments Updated (UTC)
catalyst ⬜️ Ignored (Inspect) Jul 16, 2025 6:09pm
catalyst-au ⬜️ Ignored (Inspect) Visit Preview Jul 16, 2025 6:09pm
catalyst-b2b ⬜️ Ignored (Inspect) Visit Preview Jul 16, 2025 6:09pm
catalyst-uk ⬜️ Ignored (Inspect) Visit Preview Jul 16, 2025 6:09pm

@jorgemoya jorgemoya left a comment

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.

Looks good to me, just have some questions.

}

export const build = new Command('build')
.option('--keep-temp-dir', 'Keep the temporary directory after the build')

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.

Should we set the pattern to use kebab case for all options? In preference over camel case?

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.

Yeah, POSIX standardized on --kebab-case convention, so it's what people are used to IMO. Commander automatically transforms those to camel case too:

Multi-word options such as "--template-engine" are camel-cased, becoming program.opts().templateEngine etc.

Comment on lines +3 to +5
import { cp, mkdtemp, rm } from 'node:fs/promises';
import { tmpdir } from 'node:os';
import { join, relative, sep } from 'node:path';

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.

Curious to understand why we use these libs instead of using path or fs?

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.

Not required, I just do it to explicitly indicate a built-in Node.js module and avoid ambiguity between built-ins and potentially conflicting userland modules (like https://www.npmjs.com/package/test vs node:test)

await expect(access(path)).rejects.toThrow();
});

test('cleanup is idempotent', async () => {

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.

idempotent TIL

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.

Yeah, in other words, cleanup() should not fail on the second call


import { createFilter, mkTempDir } from '../../src/commands/build';

describe('mkTempDir', () => {

@jorgemoya jorgemoya Jul 16, 2025

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.

Should we wrap all describes under the build command describe?

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.

Chatted offline, will leave as is until it gets unwieldy

@matthewvolk
matthewvolk force-pushed the CATALYST-1386-copy-to-temp branch from 92f5785 to 91d0d22 Compare July 16, 2025 18:07
@matthewvolk
matthewvolk added this pull request to the merge queue Jul 16, 2025
Merged via the queue into canary with commit 778bd21 Jul 16, 2025
12 checks passed
@matthewvolk
matthewvolk deleted the CATALYST-1386-copy-to-temp branch July 16, 2025 18:25
bc-victor added a commit that referenced this pull request Aug 7, 2025
…rtal (#2504)

* Version Packages (`canary`) (#2452)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

* Bump next to latest canary (#2465)

* docs: how to verify new merge base established when syncing (#2451)

Improve explanation around the importance of establishing a new merge base every time `canary` is merged into `integrations/makeswift`

* chore(core): remove runtime edge declarations (#2464)

* Update translations (#2468)

* feat(other): LOCAL-1444 delivery translation

* chore(core): create translations patch

---------

Co-authored-by: bc-svc-local <bc-svc-local@users.noreply.github.com>
Co-authored-by: Jorge Moya <jorge.moya@bigcommerce.com>

* Use min and max purchase quantity to set bounds of quantity number input (#2474)

* chore: update ci to include @bigcommerce/catalyst (#2463)

* chore: update deps mostly related to packages/cli (#2461)

* chore: use @commander-js/extra-typings as devDep (#2462)

* feat: render scripts from Script Manager (#2477)

* chore: hoist eslint plugins for ide support (#2460)

* feat: recursively copy cwd into temp folder (#2479)

* feat(cli): scaffold deploy command and generate bundle zip (#2481)

* feat(cli): scaffold deploy command and generate bundle zip

* feat: remove archiver and only use adm-zip

* feat: add extra checks for build files

* refactor: move checks around

* fix: use .bigcommerce/dist as build path

* refactor: split mkTempDir to own lib file and update tests

* fix: remove file

* fix: set default value for prefix

* fix: remove extra-typings

* feat: install and build all deps required to bundle catalyst (#2486)

* chore: bump opennextjs-cloudflare version (#2487)

* feat(cli): generate upload signature and upload bundle.zip (#2484)

* feat(cli): generate upload signature and upload bundle.zip

* feat: read from env variables

* feat: use zod to validate response

* refactor: parse

* feat: read api host from env

* refactor: infer option types in deploy command action (#2490)

* feat: copy templates from cli to catalyst build tmpdir (#2489)

* Version Packages (`canary`) (#2466)

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>

---------

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Co-authored-by: Nathan Booker <bookernath@users.noreply.github.com>
Co-authored-by: Matthew Volk <matt.volk@bigcommerce.com>
Co-authored-by: Jorge Moya <jorge.moya@bigcommerce.com>
Co-authored-by: bc-svc-local <102379007+bc-svc-local@users.noreply.github.com>
Co-authored-by: bc-svc-local <bc-svc-local@users.noreply.github.com>
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.

2 participants