[BD-46] feat: add paragon-cli #2460

monteri · 2023-07-20T18:10:34Z

Description

Add paragon CLI that will provide Paragon consumers with paragon <command>. Right now it allows to run paragon install-theme and specify a custom brand package to install with the --no-save option (e.g., @edx/brand-edx.org@latest) to install custom brands during MFE local development without changing package.json.

Deploy Preview

N/A

Merge Checklist

If your update includes visual changes, have they been reviewed by a designer? Send them a link to the Netlify deploy preview, if applicable.
Does your change adhere to the documented style conventions?
Do any prop types have missing descriptions in the Props API tables in the documentation site (check deploy preview)?
Were your changes tested using all available themes (see theme switcher in the header of the deploy preview, under the "Settings" icon)?
Were your changes tested in the example app?
Is there adequate test coverage for your changes?
Consider whether this change needs to reviewed/QA'ed for accessibility (a11y). If so, please add wittjeff and adamstankiewicz as reviewers on this PR.

Post-merge Checklist

Verify your changes were released to NPM at the expected version.
If you'd like, share your contribution in #show-and-tell.
🎉 🙌 Celebrate! Thanks for your contribution.

openedx-webhooks · 2023-07-20T18:10:39Z

Thanks for the pull request, @monteri!

When this pull request is ready, tag your edX technical lead.

netlify · 2023-07-20T18:10:41Z

✅ Deploy Preview for paragon-openedx ready!

Built without sensitive environment variables

Name	Link
🔨 Latest commit	`838e9e6`
🔍 Latest deploy log	https://app.netlify.com/sites/paragon-openedx/deploys/64df42d877ee5300086d9cad
😎 Deploy Preview	https://deploy-preview-2460--paragon-openedx.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

adamstankiewicz · 2023-07-21T12:35:12Z

paragon-cli.js

+
+const themes = [
+  { name: '@edx/brand-openedx', value: '' },
+  { name: '@edx/brand-edx.org', value: 'npm:@edx/brand-edx.org@latest' },


[suggestion] I believe we may want to avoid hardcoding @brand-edx.org in this specific case. Take consumers other than edX.org in the community that have their own @edx/brand packages. The CLI tool should be able to handle any @edx/brand package provided by the consumer, at the version they specify.

Q: "What @edx/brand package do you want to install?"

A: @edx/brand-edx.org@latest

Updated to this variant. Also I think it will useful to have commands in own js files. So I updated CLI to use Object instead of switch case

adamstankiewicz · 2023-07-21T12:36:05Z

paragon-cli.js

@@ -0,0 +1,57 @@
+#!/usr/bin/env node
+
+const inquirer = require('inquirer');


Oooh neat, I was looking at inquirer awhile back. Seems pretty cool, and more powerful than the commander package we've previously been using to build CLI tools.

adamstankiewicz

This looks like a great start to the foundations of a single Paragon CLI tool that supports multiple commands/features! 😄

A few additional feedback items / thoughts:

In the example app, I noticed that @edx/brand-openedx is installed / imported from directly rather than the expected @edx/brand alias. I believe we'll want to install @edx/brand-openedx in the example MFE as follows: "@edx/brand": "npm:@edx/brand-openedx@^1.1.0"
Similarly, the example/src/index.scss file will need to be updated to import from the @edx/brand alias.
Add a "start:with-theme" NPM script in the example MFE's package.json file in order to use the command as if you were an engineer in a consuming MFE, something like:

{
  "scripts": {
    "install-brand": "../bin/paragon-scripts.js theme",
    "start:with-brand": "npm run install-theme && npm start"
  }
}

Usage in example MFE

npm run start:with-theme

adamstankiewicz · 2023-07-23T11:57:43Z

package.json

@@ -8,6 +8,9 @@
  "publishConfig": {
    "access": "public"
  },
+  "bin": {
+    "paragon": "paragon-cli.js"


[suggestion] Might be good to match the folder structure of @edx/frontend-build regarding its fedx-scripts, i.e. let's move paragon-cli.js under a bin directory (and maybe even rename to paragon-scripts?) such that this line becomes something more like:

{ "bin": { "paragon": "./bin/paragon-scripts.js" } }

Yes, I agree, updated name and location

adamstankiewicz · 2023-07-23T12:04:30Z

paragon-cli.js

+  const [command, ...args] = process.argv.slice(2);
+
+  switch (command) {
+    case 'theme':


[suggestion] Perhaps install-theme to be a bit more explicit/descriptive in the command name?

Agree, changed

adamstankiewicz · 2023-07-23T12:06:54Z

paragon-cli.js

+}
+
+function installTheme(theme) {
+  const installCommand = `npm install "@edx/brand@${theme}" --no-save`;


[suggestion] This can totally be deferred, but an idea: make the --no-save option configurable/overrideable such that the command can be run where this npm install does save. The default should be --no-save, though.

I'm thinking of a possible use case where the Paragon CLI could be used in infrastructure/deployment code to install the @edx/brand aliases rather than the infrastructure/deployment code needing to do the npm install themselves (might be easier to simply run paragon install-theme during the build+deploy process 🤷‍♂️). Ah, but in this case, the user input wouldn't be possible.

Let's defer on this comment for now but come back to it at some point. I think to support the infrastructure/deployment code use case, we'd need a variant of the paragon install-theme CLI command that allowed its user to pass --brand (or similar) as an argument to avoid needing to provide user input.

Yes, I think it is good idea, maybe flag -d, --deploy or something will solve the problem and will make it clear for the user. Also I created issue for your three comments that won't be part of this PR. #2483

Thanks for creating the issue!

adamstankiewicz · 2023-07-23T12:08:43Z

paragon-cli.js

+      break;
+
+    default:
+      console.log('Unknown command. Usage: paragon <command>');


[suggestion] Take a look at the chalk JS library: https://github.com/chalk/chalk

I've started to introduce it into @edx/frontend-build's fedx-scripts CLI tool and think it's a worthwhile pattern to bring here. Basically, chalk allows you to draw more attention to console.log statements such as this by formatting it.

In this case, given it's an error, perhaps its formatted with red and bold, for example.

Added usage of chalk. For both errors I added red and bold color.

adamstankiewicz · 2023-07-23T12:10:02Z

paragon-cli.js

+      break;
+
+    default:
+      console.log('Unknown command. Usage: paragon <command>');


[curious/suggestion] Is there a way to list out all possible CLI commands as a hint for users on what commands are available to use? In a similar vein, is there or should there be a paragon help type of functionality? This, too, could likely be deferred as a separate feature addition.

Also put this comment as part of the issue #2483

adamstankiewicz · 2023-07-23T12:17:44Z

paragon-cli.js

+(async () => {
+  const [command, ...args] = process.argv.slice(2);
+
+  switch (command) {


[suggestion] To have better observability in how consumers make use of these commands, I might recommend we try dispatching a Segment event describing which command is used and how, similar to how we have sendTrackInfo in the component generator tool.

Speaking of the component generator, we may also want to backlog an issue to migrate its current implementation into this new Paragon CLI as a distinct command (not in scope for this PR).

I put this comment as part of the issue #2483

monteri · 2023-07-27T14:23:46Z

{
  "scripts": {
    "install-brand": "../bin/paragon-scripts.js theme",
    "start:with-brand": "npm run install-theme && npm start"
  }
}

We are able to use paragon command inside example so I added both example's start:with-theme command and example:start:with-theme command in the root.

adamstankiewicz · 2023-07-28T13:16:41Z

@monteri [inform] Looks like there's some linting errors.

For the one's related to "should be listed in the project's dependencies, not devDependencies", we may want to ignore this class of errors in the bin directory?

monteri · 2023-08-01T10:48:41Z

Thank you, I fixed linting errors.

codecov · 2023-08-01T10:54:20Z

Codecov Report

Patch coverage has no change and project coverage change: +0.26% 🎉

Comparison is base (d55001f) 91.38% compared to head (838e9e6) 91.65%.
Report is 53 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2460      +/-   ##
==========================================
+ Coverage   91.38%   91.65%   +0.26%     
==========================================
  Files         234      236       +2     
  Lines        4157     4195      +38     
  Branches     1001     1012      +11     
==========================================
+ Hits         3799     3845      +46     
+ Misses        351      346       -5     
+ Partials        7        4       -3

see 9 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

adamstankiewicz

Looks good! Added a minor question, and a comment about the example NPM script not seeming to work without a modification. Once those items are clarified/resolved, ✅

adamstankiewicz · 2023-08-03T19:10:58Z

bin/paragon-scripts.js

+const COMMANDS = {
+  'install-theme': themeCommand,
+  // eslint-disable-next-line no-console
+  test: () => console.log('Executing "paragon test" function...'),


[curious] Do we intend to keep this as part of the CLI?

adamstankiewicz · 2023-08-03T19:12:22Z

example/package.json

@@ -10,12 +10,13 @@
    "precommit": "npm run lint",
    "snapshot": "fedx-scripts jest --updateSnapshot",
    "start": "fedx-scripts webpack-dev-server --progress",
+    "start:with-theme": "paragon install-theme && npm start",


When running this, I get the following error:

❯ npm run start:with-theme > example@1.0.0 start:with-theme > paragon install-theme && npm start sh: paragon: command not found npm ERR! Lifecycle script `start:with-theme` failed with error: npm ERR! Error: command failed npm ERR! in workspace: example@1.0.0 npm ERR! at location: /Users/astankiewicz/Desktop/edx/paragon/example

I believe we could resolve this error by changing the line to:

../bin/paragon-scripts.js install-theme && npm start

adamstankiewicz · 2023-08-11T12:48:49Z

package.json

+    "chalk": "^4.1.2",
+    "child_process": "^1.0.2",


[curious] I have a hunch these dependencies will need to be installed of regular dependencies, including inquirer below.

If installed as devDependencies, their code won't be included in the installed bundle in the consuming application, which could lead to not being able to run the Paragon CLI correctly. It's not a concern within the example app in the Paragon repo itself, because these packages are installed into the root node_modules via the devDependencies. However, for MFEs outside of the Paragon repo, this could be a concern.

I wonder if it's worth a sanity check of trying to run the Paragon CLI outside of the example MFE app, too? Perhaps installing this branch of Paragon into frontend-template-application would be a good test (I think the bin script should work?).

npm install "https://github.com/openedx/paragon.git#raccoongang:zadorozhnii/paragon-cli-for-theming" --save

Note this also related to the // eslint-disable-next-line import/no-extraneous-dependencies comments.

adamstankiewicz · 2023-08-11T13:33:26Z

bin/install-theme.js

+      type: 'input',
+      name: 'theme',
+      message: 'What @edx/brand package do you want to install?',
+      default: '',


nit: Should this default @edx/brand-openedx@latest? Or a required field? Worth a sanity check.

openedx-webhooks · 2023-08-18T11:22:46Z

@monteri 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

edx-semantic-release · 2023-08-18T12:04:37Z

🎉 This PR is included in version 21.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Exposes a `paragon` CLI with an `install-theme` command. Example usage in `package.json`: ``` { "scripts": { "start:with-theme": "paragon install-theme && npm start", "build:with-theme": "paragon install-theme && npm run build" } } ``` The Paragon CLI will prompt from which NPM package you wish to install as the `@edx/brand` package (e.g., `@edx/brand-edx.org@latest`), and install it without modifying the `package.json` or `package-lock.json` files. Additional commands will be added to the `paragon` CLI tool in the future.

feat: add paragon-cli

4c6dbb2

openedx-webhooks added the blended PR is managed through 2U's blended developmnt program label Jul 20, 2023

monteri requested review from adamstankiewicz and PKulkoRaccoonGang July 20, 2023 18:10

monteri marked this pull request as draft July 20, 2023 18:11

monteri self-assigned this Jul 20, 2023

monteri mentioned this pull request Jul 21, 2023

Ensure consuming projects install @edx/brand as the @edx/brand-openedx package by default #2240

Closed

adamstankiewicz reviewed Jul 21, 2023

View reviewed changes

adamstankiewicz mentioned this pull request Jul 23, 2023

Migrate off of compound components to standalone components #2464

Open

adamstankiewicz reviewed Jul 23, 2023

View reviewed changes

monteri marked this pull request as ready for review July 27, 2023 13:34

monteri added 2 commits July 27, 2023 17:54

refactor: clean up the CLI tool

0ea5d11

refactor: rename

a0b54a9

monteri mentioned this pull request Jul 27, 2023

CLI improvements #2483

Closed

fix: linting

a4641b2

PKulkoRaccoonGang approved these changes Aug 3, 2023

View reviewed changes

adamstankiewicz reviewed Aug 3, 2023

View reviewed changes

fix: PR comments update

da4b777

adamstankiewicz linked an issue Aug 11, 2023 that may be closed by this pull request

Discovery: Paragon CLI/Migrator #2242

Closed

adamstankiewicz reviewed Aug 11, 2023

View reviewed changes

fix: move dependencies to prod

838e9e6

adamstankiewicz approved these changes Aug 18, 2023

View reviewed changes

adamstankiewicz merged commit 33a7c74 into openedx:master Aug 18, 2023
9 checks passed

edx-semantic-release added the released label Aug 18, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BD-46] feat: add paragon-cli #2460

[BD-46] feat: add paragon-cli #2460

monteri commented Jul 20, 2023 •

edited by adamstankiewicz

Loading

openedx-webhooks commented Jul 20, 2023 •

edited

Loading

netlify bot commented Jul 20, 2023 •

edited

Loading

adamstankiewicz Jul 21, 2023

adamstankiewicz Jul 21, 2023

monteri Jul 27, 2023

adamstankiewicz Jul 21, 2023

monteri Jul 27, 2023

adamstankiewicz left a comment

adamstankiewicz Jul 23, 2023

monteri Jul 27, 2023

adamstankiewicz Jul 23, 2023

monteri Jul 27, 2023

adamstankiewicz Jul 23, 2023 •

edited

Loading

monteri Jul 27, 2023

adamstankiewicz Jul 28, 2023

adamstankiewicz Jul 23, 2023 •

edited

Loading

monteri Jul 27, 2023

adamstankiewicz Jul 23, 2023

monteri Jul 27, 2023

adamstankiewicz Jul 23, 2023 •

edited

Loading

monteri Jul 27, 2023

monteri commented Jul 27, 2023

adamstankiewicz commented Jul 28, 2023

monteri commented Aug 1, 2023 •

edited

Loading

codecov bot commented Aug 1, 2023 •

edited

Loading

adamstankiewicz left a comment

adamstankiewicz Aug 3, 2023

adamstankiewicz Aug 3, 2023

adamstankiewicz Aug 11, 2023

adamstankiewicz Aug 11, 2023

openedx-webhooks commented Aug 18, 2023

edx-semantic-release commented Aug 18, 2023

		@@ -0,0 +1,57 @@
		#!/usr/bin/env node

		const inquirer = require('inquirer');

[BD-46] feat: add paragon-cli #2460

[BD-46] feat: add paragon-cli #2460

Conversation

monteri commented Jul 20, 2023 • edited by adamstankiewicz Loading

Description

Deploy Preview

Merge Checklist

Post-merge Checklist

openedx-webhooks commented Jul 20, 2023 • edited Loading

netlify bot commented Jul 20, 2023 • edited Loading

✅ Deploy Preview for paragon-openedx ready!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamstankiewicz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamstankiewicz Jul 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamstankiewicz Jul 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

adamstankiewicz Jul 23, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

monteri commented Jul 27, 2023

adamstankiewicz commented Jul 28, 2023

monteri commented Aug 1, 2023 • edited Loading

codecov bot commented Aug 1, 2023 • edited Loading

Codecov Report

adamstankiewicz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

openedx-webhooks commented Aug 18, 2023

edx-semantic-release commented Aug 18, 2023

monteri commented Jul 20, 2023 •

edited by adamstankiewicz

Loading

openedx-webhooks commented Jul 20, 2023 •

edited

Loading

netlify bot commented Jul 20, 2023 •

edited

Loading

adamstankiewicz Jul 23, 2023 •

edited

Loading

adamstankiewicz Jul 23, 2023 •

edited

Loading

adamstankiewicz Jul 23, 2023 •

edited

Loading

monteri commented Aug 1, 2023 •

edited

Loading

codecov bot commented Aug 1, 2023 •

edited

Loading