-
Notifications
You must be signed in to change notification settings - Fork 36
feat(cli): create cli package with config, plugins and commands #6
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
700ab69
6b65be3
3cb5aa9
b4b7011
8af3e5e
a298368
bfddaf3
8c067f0
77cd96a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,3 @@ | ||
| { | ||
| "recommendations": [ | ||
| "nrwl.angular-console", | ||
| "esbenp.prettier-vscode", | ||
| ] | ||
| "recommendations": ["nrwl.angular-console", "esbenp.prettier-vscode"] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| # cli | ||
|
|
||
| This library was generated with [Nx](https://nx.dev). | ||
|
|
||
| ## Building | ||
|
|
||
| Run `nx build cli` to build the library. | ||
|
|
||
| ## Running unit tests | ||
|
|
||
| Run `nx test cli` to execute the unit tests via [Jest](https://jestjs.io). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| const baseConfig = require('../../eslint.config.js'); | ||
|
|
||
| module.exports = [ | ||
| ...baseConfig, | ||
| { | ||
| files: ['**/*.json'], | ||
| rules: { | ||
| '@nx/dependency-checks': [ | ||
| 'error', | ||
| { | ||
| ignoredFiles: ['{projectRoot}/eslint.config.{js,cjs,mjs}'], | ||
| // TODO: @nx/dependency-checks incorrectly reports unused dependencies | ||
| checkObsoleteDependencies: false, | ||
| }, | ||
| ], | ||
| }, | ||
| languageOptions: { | ||
| parser: require('jsonc-eslint-parser'), | ||
| }, | ||
| }, | ||
| ]; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,15 @@ | ||
| { | ||
| "name": "@callstack/rnef-cli", | ||
| "version": "0.0.1", | ||
| "dependencies": { | ||
| "commander": "^12.1.0", | ||
| "tslib": "^2.3.0" | ||
| }, | ||
| "bin": { | ||
| "rnef": "./src/bin.js" | ||
| }, | ||
| "type": "commonjs", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Isn't this the default? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nx template I guess |
||
| "main": "./src/index.js", | ||
| "typings": "./src/index.d.ts", | ||
| "private": true | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: This can leave as a separate TBD, but I think, if we plan to eventually publish each separate module to npm, we should take care of adding all necessary fields to the |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| { | ||
| "name": "cli", | ||
| "$schema": "../../node_modules/nx/schemas/project-schema.json", | ||
| "sourceRoot": "packages/cli/src", | ||
| "projectType": "library", | ||
| "tags": [], | ||
| "targets": { | ||
| "build": { | ||
| "executor": "@nx/js:tsc", | ||
| "outputs": ["{options.outputPath}"], | ||
| "options": { | ||
| "outputPath": "packages/cli/dist", | ||
| "main": "packages/cli/src/index.ts", | ||
| "tsConfig": "packages/cli/tsconfig.lib.json", | ||
| "assets": ["packages/cli/*.md"] | ||
| } | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| #!/usr/bin/env node | ||
|
|
||
| import { cli } from './lib/cli'; | ||
|
|
||
| cli(); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,2 @@ | ||
| export * from './lib/cli'; | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| type PlatformConfig = { | ||
| name: string; | ||
| init: () => Promise<void>; | ||
| build: () => Promise<void>; | ||
| run: () => Promise<void>; | ||
| linkModules: () => Promise<void>; | ||
| linkAssets: () => Promise<void>; | ||
| }; | ||
|
|
||
| type PluginOutput = { | ||
| name: string; | ||
| description: string; | ||
| action: () => Promise<void>; | ||
| platform?: () => PlatformConfig; | ||
| }; | ||
|
|
||
| type CommandType = { | ||
| name: string; | ||
| description: string; | ||
| action: () => void; | ||
| }; | ||
|
|
||
| type PluginArgs = { | ||
| registerCommand: (command: CommandType) => void; | ||
| }; | ||
|
|
||
| export default function SamplePlugin(api: PluginArgs): PluginOutput { | ||
| const name = 'xplat'; | ||
| const linkModules = async () => { | ||
| // noop | ||
| }; | ||
| const linkAssets = async () => { | ||
| // noop | ||
| }; | ||
| const build = async () => { | ||
| linkModules(); | ||
| linkAssets(); | ||
| // noop - build | ||
| }; | ||
| const run = async () => { | ||
| linkModules(); | ||
| linkAssets(); | ||
| // noop - build | ||
| }; | ||
|
|
||
| api.registerCommand({ | ||
| name: `${name}build`, | ||
| description: 'Build xplat', | ||
| action: build, | ||
| }); | ||
|
|
||
| api.registerCommand({ | ||
| name: `${name}run`, | ||
| description: 'Run xplat', | ||
| action: run, | ||
| }); | ||
|
|
||
| return { | ||
| name: 'xplat', | ||
| description: 'sample plugin', | ||
| action: async () => { | ||
| // noop | ||
| }, | ||
| }; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| import { describe, it, expect } from 'vitest'; | ||
| import { cli } from '../cli'; | ||
|
|
||
| describe('cli', () => { | ||
| it('should throw when config not found', async () => { | ||
| await expect(cli()).rejects.toThrow('rnef.config not found in any parent directory of /'); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: Do we expect the config to be always defined, a/k/a no defaults? Why? |
||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,60 @@ | ||
| import { vi, expect, beforeEach, afterEach, test } from 'vitest'; | ||
| import { getConfig } from '../getConfig'; | ||
| import { | ||
| cleanup, | ||
| writeFiles, | ||
| getTempDirectory, | ||
| } from '../../../../../test-helpers'; | ||
|
|
||
| const DIR = getTempDirectory('test_config'); | ||
|
|
||
| beforeEach(() => { | ||
| cleanup(DIR); | ||
| vi.resetModules(); | ||
| vi.clearAllMocks(); | ||
| }); | ||
|
|
||
| afterEach(() => cleanup(DIR)); | ||
|
|
||
| test.each([['.js'], ['.mjs'], ['.ts']])( | ||
| 'should load configs with %s extension', | ||
| async (ext) => { | ||
| writeFiles(DIR, { | ||
| [`rnef.config${ext}`]: `module.exports = { | ||
| plugins: {} | ||
| }`, | ||
| }); | ||
| expect(await getConfig(DIR)).toMatchObject({ commands: [] }); | ||
| } | ||
| ); | ||
|
|
||
| test('should load plugin that registers a command', async () => { | ||
| writeFiles(DIR, { | ||
| 'rnef.config.js': `module.exports = { | ||
| plugins: { | ||
| 'test-plugin': function TestPlugin(config) { | ||
| return { | ||
| name: 'test-plugin', | ||
| commands: [ | ||
| { | ||
| name: 'test-command', | ||
| description: 'Test command', | ||
| action: () => { console.log('Test command executed'); }, | ||
| }, | ||
| ], | ||
| }; | ||
| } | ||
| } | ||
| }`, | ||
| }); | ||
|
|
||
| expect(await getConfig(DIR)).toMatchObject({ | ||
| commands: [ | ||
| { | ||
| name: 'test-command', | ||
| description: 'Test command', | ||
| action: expect.any(Function), | ||
| }, | ||
| ], | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,36 @@ | ||
| import { Command } from 'commander'; | ||
| import { version } from '../../package.json'; | ||
| import { getConfig } from '@callstack/rnef-config'; | ||
|
|
||
| const program = new Command(); | ||
|
|
||
| program | ||
| .name('rnef') | ||
| .description('React Native Enterprise Framework CLI.') | ||
| .version(version); | ||
|
|
||
| export const cli = async () => { | ||
| const config = await getConfig(); | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. How do we plan to handle case when two plugins define the same command? Last one wins or some error/warning to the user? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Initially I thought that for now it's ok that last one wins and module.exports = {
plugins: {
pluginAndroid,
pluginApple,
android2: otherAndroidPlugin, // <-- when defined this way we could prefix commands with `android2:`
commands: { // <-- if we keep the commands in user config then this could also be possible
[pluginAndroid.commands.build]: CustomCommand
}
}
}There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When we last discussed the (I generally think it is hard to solve) |
||
|
|
||
| // Register commands from the config | ||
| config.commands?.forEach((command) => { | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: One of the design decisions I made in In the current design, I think this is good opportunity to talk about this. I briefly touch-based on it on Slack, but I think we can move that discussion over to here. |
||
| const cmd = program | ||
| .command(command.name) | ||
| .description(command.description || '') | ||
| .action(async () => { | ||
| try { | ||
| command.action(program.args); | ||
| } catch (e) { | ||
| // TODO handle nicely | ||
| console.log('Error: ', e); | ||
| process.exit(1); | ||
| } | ||
| }); | ||
|
|
||
| for (const opt of command.options || []) { | ||
| cmd.option(opt.name, opt.description ?? ''); | ||
| } | ||
| }); | ||
|
|
||
| program.parse(); | ||
| }; | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| { | ||
| "extends": "../../tsconfig.base.json", | ||
| "compilerOptions": { | ||
| "module": "commonjs", | ||
| "forceConsistentCasingInFileNames": true, | ||
| "strict": true, | ||
| "noImplicitOverride": true, | ||
| "noPropertyAccessFromIndexSignature": true, | ||
| "noImplicitReturns": true, | ||
| "noFallthroughCasesInSwitch": true, | ||
| "resolveJsonModule": true | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: if we extend the base, shouldn't we put all these into the base already? Some of these don't seem like CLI specific, such as |
||
| }, | ||
| "files": [], | ||
| "include": [], | ||
| "references": [ | ||
| { | ||
| "path": "./tsconfig.lib.json" | ||
| }, | ||
| { | ||
| "path": "./tsconfig.spec.json" | ||
| } | ||
| ] | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,10 @@ | ||
| { | ||
| "extends": "./tsconfig.json", | ||
| "compilerOptions": { | ||
| "outDir": "../../dist/out-tsc", | ||
| "declaration": true, | ||
| "types": ["node"] | ||
| }, | ||
| "include": ["src/**/*.ts", "../config/src/lib/config.ts"], | ||
| "exclude": ["vitest.config.ts", "src/**/*.spec.ts", "src/**/*.test.ts"] | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,14 @@ | ||
| { | ||
| "extends": "./tsconfig.json", | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: This is more of a question related to Nx, than this PR. What is the purpose of both There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sa far as I undestand it:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should discuss whether we want to go with Nx defaults (probably would be easier for us), or to deliberately set the things up in the way want, but to make sure we do it consistently between the projects |
||
| "compilerOptions": { | ||
| "outDir": "../../dist/out-tsc", | ||
| "module": "commonjs", | ||
| "types": ["node"] | ||
| }, | ||
| "include": [ | ||
| "vite.config.ts", | ||
| "src/**/*.test.ts", | ||
| "src/**/*.spec.ts", | ||
| "src/**/*.d.ts" | ||
| , "__tests__/getConfig.spec.ts" ] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This was autogenerated by Nx I guess. I think it makes more sense to put There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. +1 to |
||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,20 @@ | ||
| import { defineConfig } from 'vitest/config'; | ||
|
|
||
| export default defineConfig({ | ||
| test: { | ||
| name: 'cli', | ||
| environment: 'node', | ||
| include: ['**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'], | ||
| coverage: { | ||
| provider: 'v8', | ||
| reporter: ['text', 'json', 'html'], | ||
| reportsDirectory: '../../coverage/packages/cli', | ||
| }, | ||
| typecheck: { | ||
| tsconfig: './tsconfig.spec.json', | ||
| }, | ||
| }, | ||
| resolve: { | ||
| extensions: ['.ts', '.js', '.html'], | ||
| }, | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| # config | ||
|
|
||
| This library was generated with [Nx](https://nx.dev). | ||
|
|
||
| ## Building | ||
|
|
||
| Run `nx build config` to build the library. | ||
|
|
||
| ## Running unit tests | ||
|
|
||
| Run `nx test config` to execute the unit tests via [Vitest](https://vitest.dev/). |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| const baseConfig = require('../../eslint.config.js'); | ||
|
|
||
| module.exports = [ | ||
| ...baseConfig, | ||
| { | ||
| files: ['**/*.json'], | ||
| rules: { | ||
| '@nx/dependency-checks': [ | ||
| 'error', | ||
| { | ||
| ignoredFiles: [ | ||
| '{projectRoot}/eslint.config.{js,cjs,mjs}', | ||
| '{projectRoot}/vite.config.{js,ts,mjs,mts}', | ||
| ], | ||
| }, | ||
| ], | ||
| }, | ||
| languageOptions: { | ||
| parser: require('jsonc-eslint-parser'), | ||
| }, | ||
| }, | ||
| ]; |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| { | ||
| "name": "@callstack/rnef-config", | ||
| "version": "0.0.1", | ||
| "dependencies": { | ||
| "tslib": "^2.3.0" | ||
| }, | ||
| "type": "commonjs", | ||
| "main": "./src/index.js", | ||
| "typings": "./src/index.d.ts", | ||
| "private": true | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.