feat(dev-cli): Add clerk-dev CLI tool by dstaley · Pull Request #3689 · clerk/javascript

dstaley · 2024-07-11T07:13:08Z

Description

This PR adds the @clerk/dev package containing the clerk-dev CLI tool, which is intended to simplify the workflow for iterating on packages in this repo within sample applications. More information including available commands can be found in the README.

This initial version is compatible with Next.js and Vite, with more frameworks to come in the future.

Checklist

npm test runs as expected.
npm run build runs as expected.
(If applicable) JSDoc comments have been added or updated for any package exports
(If applicable) Documentation has been updated

Type of change

changeset-bot · 2024-07-11T07:13:12Z

🦋 Changeset detected

Latest commit: 285bc39

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@clerk/dev-cli	Patch

Not sure what this means? Click here to learn what changesets are.

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

LekoArts · 2024-07-11T10:11:15Z

+        fapiUrl: 'REPLACE_WITH_FAPI_URL',
+        bapiUrl: 'https://api.clerk.com',


Do we need these?

I put these in since these values are displayed on the configuration screen in the dashboard. I'm not totally sure of when a user would need to have these values available.

Then let's remove it for now 👍

LekoArts · 2024-07-11T10:13:26Z

+  if (process.env.VISUAL) {
+    spawn(process.env.VISUAL, [CONFIG_FILE], {
+      stdio: 'inherit',
+      env: {
+        ...process.env,
+      },
+    });
+    console.log(`Configuration file written to ${CONFIG_FILE}.`);
+  } else if (process.env.EDITOR) {
+    spawn(process.env.EDITOR, [CONFIG_FILE], {
+      stdio: 'inherit',
+      env: {
+        ...process.env,
+      },
+    });
+    console.log(`Configuration file written to ${CONFIG_FILE}.`);
+  } else {
+    console.log(`Configuration file written to ${CONFIG_FILE}. Replace with your values.`);
+  }


The README didn't mention this functionality. So it tries to open the editor so that you edit the secretKey/publishableKey.

Alternatively, you could also ask for these two values (+ the name for an instance) during init and write them to the file directly. Then it's all happening in the CLI

In a future iteration the plan is to have this automatically configured by the clerk-dev auth command, so I didn't want to spend the time setting up a prompt-based configuration. However it's looking like clerk-dev auth might be a ways off, so it might be worth exploring in a follow-up PR.

LekoArts · 2024-07-11T10:14:54Z

+ */
+export async function setup({ js = true }) {
+  console.log('Installing monorepo versions of Clerk packages from package.json...');
+  await linkDependencies();


I thought we wanted to support other tools (e.g. secco or yalc). Wouldn't this step be better placed into watch so that I can still run setup but use e.g. secco for the rest of the flow?

I've added a --skip-install option to allow for this! The idea is that clerk-dev setup will intelligently choose the right installation mechanism depending on framework. Next.js and Vite seem to work fine with npm install, so that's the default at the moment, but once we add support for other frameworks you'll see this command branch into other tools like secco.

But, in the event you want the framework configuration but not the dependency configuration, you can now use clerk-dev setup --skip-install.

LekoArts · 2024-07-11T11:37:37Z

+    .option('--js', 'only start the watcher for clerk-js')
+    .option('--no-js', 'do not spawn the clerk-js watcher (macOS only)')


These two boolean flags feel a bit awkward, what happens if in the future one wants to not spawn other things?

How about replacing this with an --ignore flag that accepts multiple values?

Then the default is that clerk-js is built, too, and one can use --ignore to not build it or other stuff

With --no-js, we can align with the flags passed to clerk-dev setup. So for example, clerk-dev setup --no-js would be followed by clerk-dev watch --no-js. On Linux you could do clerk-dev setup --no-js followed by clerk-dev watch since watch doesn't spawn the clerk-js builder on Linux. I can definitely see the awkwardness when you factor in the Linux implementation, but given our entire team uses macOS I felt that was acceptable. Is there another flag that would be less awkward but retain the alignment with clerk-dev setup so that users don't need to learn two different flags?

I think the --ignore flag is a good idea, but is something I'd like to save for later once we have a better idea of what situations would cause a developer to want to ignore other packages.

Co-authored-by: Lennart <lekoarts@gmail.com>

LekoArts

Marking as "Approve" as generally I think it's mostly good to go. So you're unblocked if you want to merge today.

Left some two smaller comments on this new review iteration.

As for the older comments:

I'd like us to publish it but wouldn't mind doing that at a later stage if we don't want to do it just now
--js and --no-js still feels awkward as mentioned

LekoArts · 2024-07-12T06:42:25Z

+You can use the `set-instance` command to switch between `activeInstance` afterwards:
+
+```shell
+clerk-dev set-instance yourName
+```


config was missing 👍

Suggested change

You can use the `set-instance` command to switch between `activeInstance` afterwards:

```shell

clerk-dev set-instance yourName

```

You can use the `set-instance` command to switch between `activeInstance` afterwards:

```shell

clerk-dev set-instance yourName

```

If you have the `VISUAL` or `EDITOR` environment variable set in your shell, you can open the configuration like so:

```shell

clerk-dev config

```

LekoArts · 2024-07-12T06:44:44Z

+export default function cli() {
+  const program = new Command();
+
+  program.name('clerk-dev').description('CLI to make developing Clerk packages easier').version('0.0.0');


Nit: Use the version from package.json to set the .version()

LekoArts · 2024-07-12T06:50:57Z

+        fapiUrl: 'REPLACE_WITH_FAPI_URL',
+        bapiUrl: 'https://api.clerk.com',


Then let's remove it for now 👍

brkalow

Co-authored-by: Lennart <lekoarts@gmail.com>

dstaley added 10 commits July 1, 2024 16:12

feat(clerk-dev): Add initial version of clerk-dev CLI

c7e353d

Merge branch 'main' into ds.feat/clerk-dev-cli

38330b3

feat(clerk-dev): Use Commander for CLI framework

781d97e

feat(clerk-dev): Add config command

0d5ba56

feat(clerk-dev): Merge setup and install steps

1bf6a8b

feat(clerk-dev): Add set-instance command

9f278cd

docs(clerk-dev): Add config command to README

56a03f0

docs(clerk-dev): Document --no-js

bd8d579

docs(clerk-dev): Add more comments

4c77f50

feat(clerk-dev): Check if env file exists before writing

ccbda33

dstaley added 2 commits July 11, 2024 00:13

Merge branch 'main' into ds.feat/clerk-dev-cli

27b3cef

chore(repo): Add blank changeset

89a2f0f

dstaley changed the title ~~feat(clerk-dev): Add clerk-dev CLI tool~~ feat(dev): Add clerk-dev CLI tool Jul 11, 2024

dstaley commented Jul 11, 2024

View reviewed changes

Comment thread packages/dev/package.json Outdated

LekoArts suggested changes Jul 11, 2024

View reviewed changes

LekoArts reviewed Jul 11, 2024

View reviewed changes

Comment thread packages/dev/src/utils/getClerkPackages.js Outdated

dstaley and others added 6 commits July 11, 2024 11:30

docs(clerk-dev): Clarify setup process

41a0cae

Co-authored-by: Lennart <lekoarts@gmail.com>

chore(dev-cli): Rename @clerk/dev to @clerk/dev-cli

592b4cd

docs(dev-cli): Add summary of package

763045d

docs(dev-cli): Clarify setup and watch commands

8b250f7

feat(dev-cli): Add --skip-install option

dddc94c

feat(dev-cli): Replace glob with globby

7616c32

dstaley changed the title ~~feat(dev): Add clerk-dev CLI tool~~ feat(dev-cli): Add clerk-dev CLI tool Jul 11, 2024

dstaley requested a review from LekoArts July 11, 2024 19:46

LekoArts approved these changes Jul 12, 2024

View reviewed changes

dstaley added 3 commits July 23, 2024 10:24

Merge branch 'main' into ds.feat/clerk-dev-cli

7ba1fa3

chore(dev-cli): Update package publishing config

87b81da

docs(dev-cli): Update installation instructions

285bc39

brkalow approved these changes Jul 23, 2024

View reviewed changes

dstaley merged commit 55a30de into main Jul 23, 2024

dstaley deleted the ds.feat/clerk-dev-cli branch July 23, 2024 18:31

clerk-cookie mentioned this pull request Jul 23, 2024

ci(repo): Version packages #3785

Merged

brkalow pushed a commit that referenced this pull request Jul 24, 2024

feat(dev-cli): Add clerk-dev CLI tool (#3689)

0ac69ee

Co-authored-by: Lennart <lekoarts@gmail.com>

		fapiUrl: 'REPLACE_WITH_FAPI_URL',
		bapiUrl: 'https://api.clerk.com',

		.option('--js', 'only start the watcher for clerk-js')
		.option('--no-js', 'do not spawn the clerk-js watcher (macOS only)')

Uh oh!

Conversation

dstaley commented Jul 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Type of change

Uh oh!

changeset-bot Bot commented Jul 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

LekoArts left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

brkalow left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dstaley commented Jul 11, 2024 •

edited

Loading

changeset-bot Bot commented Jul 11, 2024 •

edited

Loading