From fb22a5ae07a53ce0eace25fdd3831ecf899dd654 Mon Sep 17 00:00:00 2001 From: liuyi Date: Sat, 17 Jun 2023 12:49:30 +0800 Subject: [PATCH] fix(cli): incorrect behaviors (#1626) * fix(cli): target selector is not available in interactive mode * fix(cli): js binding file should export * fix(cli): wrong node engine requirements syntax * feat(cli): support esm module * restore js binding implementation in v2 --------- Co-authored-by: LongYinan --- cli/codegen/commands.ts | 6 ++ cli/docs/new.md | 1 + cli/src/api/build.ts | 31 ++++--- cli/src/api/new.ts | 1 + cli/src/api/templates/js-binding.ts | 11 ++- cli/src/api/templates/package.json.ts | 77 ++++++++++++------ cli/src/commands/new.ts | 18 ++-- cli/src/def/new.ts | 12 +++ .../__snapshots__/version.spec.ts.md | 14 ++-- .../__snapshots__/version.spec.ts.snap | Bin 364 -> 364 bytes cli/src/utils/__tests__/typegen.spec.ts | 2 +- cli/src/utils/config.ts | 13 ++- cli/src/utils/typegen.ts | 21 ++++- cli/src/utils/version.ts | 2 +- 14 files changed, 154 insertions(+), 55 deletions(-) diff --git a/cli/codegen/commands.ts b/cli/codegen/commands.ts index 2bdba2a0..33983d71 100644 --- a/cli/codegen/commands.ts +++ b/cli/codegen/commands.ts @@ -103,6 +103,12 @@ const NEW_OPTIONS: CommandSchema = { description: 'Whether to run the command in dry-run mode', default: false, }, + { + name: 'esm', + type: 'boolean', + description: 'Whether enable ESM support', + default: false, + }, ], } diff --git a/cli/docs/new.md b/cli/docs/new.md index ad17bfdb..c4341923 100644 --- a/cli/docs/new.md +++ b/cli/docs/new.md @@ -35,3 +35,4 @@ new NapiCli().new({ | enableTypeDef | --enable-type-def | boolean | false | true | Whether enable the `type-def` feature for typescript definitions auto-generation | | enableGithubActions | --enable-github-actions | boolean | false | true | Whether generate preconfigured GitHub Actions workflow | | dryRun | --dry-run | boolean | false | false | Whether to run the command in dry-run mode | +| esm | --esm | boolean | false | false | Whether enable ESM support | diff --git a/cli/src/api/build.ts b/cli/src/api/build.ts index f3a16446..11133ae6 100644 --- a/cli/src/api/build.ts +++ b/cli/src/api/build.ts @@ -28,7 +28,7 @@ import { writeFileAsync, } from '../utils/index.js' -import { createJsBinding } from './templates/index.js' +import { createCjsBinding } from './templates/index.js' const debug = debugFactory('build') @@ -452,8 +452,8 @@ class Builder { // only for cdylib if (this.cdyLibName) { - await this.generateTypeDef() - await this.writeJsBinding() + const idents = await this.generateTypeDef() + await this.writeJsBinding(idents) } return this.outputs @@ -523,12 +523,12 @@ class Builder { private async generateTypeDef() { if (!(await fileExists(this.envs.TYPE_DEF_TMP_PATH))) { - return + return [] } const dest = join(this.outputDir, this.options.dts ?? 'index.d.ts') - const dts = await processTypeDef( + const { dts, exports } = await processTypeDef( this.envs.TYPE_DEF_TMP_PATH, !this.options.noDtsHeader ? this.options.dtsHeader ?? DEFAULT_TYPE_DEF_HEADER @@ -547,21 +547,32 @@ class Builder { debug.error('Failed to write type def file') debug.error(e as Error) } + + return exports } - private async writeJsBinding() { - if (!this.options.platform || this.options.noJsBinding) { + private async writeJsBinding(idents: string[]) { + if ( + !this.options.platform || + this.options.noJsBinding || + idents.length === 0 + ) { return } - const dest = join(this.outputDir, this.options.jsBinding ?? 'index.js') + const name = parse(this.options.jsBinding ?? 'index.js').name - const js = createJsBinding(this.config.binaryName, this.config.packageName) + const cjs = createCjsBinding( + this.config.binaryName, + this.config.packageName, + idents, + ) try { + const dest = join(this.outputDir, `${name}.js`) debug('Writing js binding to:') debug(' %i', dest) - await writeFileAsync(dest, js, 'utf-8') + await writeFileAsync(dest, cjs, 'utf-8') this.outputs.push({ kind: 'js', path: dest, diff --git a/cli/src/api/new.ts b/cli/src/api/new.ts index 667226c1..185ffbd8 100644 --- a/cli/src/api/new.ts +++ b/cli/src/api/new.ts @@ -146,6 +146,7 @@ function generatePackageJson(options: NewOptions): Output { license: options.license, engineRequirement: napiEngineRequirement(options.minNodeApiVersion), cliVersion: CLI_VERSION, + esm: options.esm, }), } } diff --git a/cli/src/api/templates/js-binding.ts b/cli/src/api/templates/js-binding.ts index e5750748..f3d14d10 100644 --- a/cli/src/api/templates/js-binding.ts +++ b/cli/src/api/templates/js-binding.ts @@ -1,8 +1,10 @@ -export function createJsBinding(localName: string, pkgName: string): string { +export function createCjsBinding( + localName: string, + pkgName: string, + idents: string[], +): string { return `// prettier-ignore -/* tslint:disable */ /* eslint-disable */ - /* auto-generated by NAPI-RS */ const { existsSync, readFileSync } = require('fs') @@ -253,5 +255,8 @@ if (!nativeBinding) { throw new Error(\`Failed to load native binding\`) } +${idents + .map((ident) => `module.exports.${ident} = binding.${ident}`) + .join('\n')} ` } diff --git a/cli/src/api/templates/package.json.ts b/cli/src/api/templates/package.json.ts index efb6a4c5..ea29a607 100644 --- a/cli/src/api/templates/package.json.ts +++ b/cli/src/api/templates/package.json.ts @@ -1,3 +1,5 @@ +import { CommonPackageJsonFields } from '../../utils/config.js' + export const createPackageJson = ({ name, binaryName, @@ -5,6 +7,7 @@ export const createPackageJson = ({ license, engineRequirement, cliVersion, + esm, }: { name: string binaryName: string @@ -12,32 +15,54 @@ export const createPackageJson = ({ license: string engineRequirement: string cliVersion: string + esm: boolean }) => { - return `{ - "name": "${name}", - "version": "1.0.0", - "main": "index.js", - "types": "index.d.ts", - "license": "${license}", - "engines": { - "node": "${engineRequirement}" - }, - "napi": { - "binaryName": "${binaryName}", - "targets": [ - ${targets.map((t) => `"${t}"`).join(',\n ')} - ] - }, - "scripts": { - "test": "node -e \\"assert(require('.').sum(1, 2) === 3)\\"", - "build": "napi build --release --platform --strip", - "build:debug": "napi build", - "prepublishOnly": "napi prepublish -t npm", - "artifacts": "napi artifacts", - "version": "napi version" - }, - "devDependencies": { - "@napi-rs/cli": "^${cliVersion}" + const content: CommonPackageJsonFields = { + name, + version: '1.0.0', + license, + engines: { + node: engineRequirement, + }, + type: 'commonjs', + main: 'index.js', + types: 'index.d.ts', + module: undefined, + exports: undefined, + napi: { + binaryName, + targets, + }, + scripts: { + test: 'node -e "assert(require(\'.\').sum(1, 2) === 3)"', + build: 'napi build --release --platform --strip', + 'build:debug': 'napi build', + prepublishOnly: 'napi prepublish -t npm', + artifacts: 'napi artifacts', + version: 'napi version', + }, + devDependencies: { + '@napi-rs/cli': `^${cliVersion}`, + }, } -}` + + if (esm) { + content.type = 'module' + content.main = 'index.cjs' + content.module = 'index.js' + content.exports = { + '.': { + import: { + default: './index.js', + types: './index.d.ts', + }, + require: { + default: './index.cjs', + types: './index.d.ts', + }, + }, + } + } + + return JSON.stringify(content, null, 2) } diff --git a/cli/src/commands/new.ts b/cli/src/commands/new.ts index 13c9cf4a..0bb2ec1e 100644 --- a/cli/src/commands/new.ts +++ b/cli/src/commands/new.ts @@ -40,6 +40,7 @@ export class NewCommand extends BaseNewCommand { return { ...cmdOptions, name: await this.fetchName(path.parse(cmdOptions.path).base), + esm: await this.fetchEnableEsm(), minNodeApiVersion: await this.fetchNapiVersion(), targets: await this.fetchTargets(), license: await this.fetchLicense(), @@ -94,10 +95,6 @@ export class NewCommand extends BaseNewCommand { } private async fetchTargets(): Promise { - if (this.enableDefaultTargets) { - return DEFAULT_TARGETS.concat() - } - if (this.enableAllTargets) { return AVAILABLE_TARGETS.concat() } @@ -107,7 +104,7 @@ export class NewCommand extends BaseNewCommand { type: 'checkbox', loop: false, message: 'Choose target(s) your crate will be compiled to', - default: DEFAULT_TARGETS, + default: this.enableDefaultTargets ? DEFAULT_TARGETS : [], choices: AVAILABLE_TARGETS, }) @@ -135,4 +132,15 @@ export class NewCommand extends BaseNewCommand { return enableGithubActions } + + private async fetchEnableEsm(): Promise { + const { esm } = await inquirer.prompt({ + name: 'esm', + type: 'confirm', + message: 'Enable ESM support', + default: this.esm, + }) + + return esm + } } diff --git a/cli/src/def/new.ts b/cli/src/def/new.ts index 387eb050..d9ff01f2 100644 --- a/cli/src/def/new.ts +++ b/cli/src/def/new.ts @@ -51,6 +51,10 @@ export abstract class BaseNewCommand extends Command { description: 'Whether to run the command in dry-run mode', }) + esm = Option.Boolean('--esm', false, { + description: 'Whether enable ESM support', + }) + getOptions() { return { path: this.$$path, @@ -63,6 +67,7 @@ export abstract class BaseNewCommand extends Command { enableTypeDef: this.enableTypeDef, enableGithubActions: this.enableGithubActions, dryRun: this.dryRun, + esm: this.esm, } } } @@ -127,6 +132,12 @@ export interface NewOptions { * @default false */ dryRun?: boolean + /** + * Whether enable ESM support + * + * @default false + */ + esm?: boolean } export function applyDefaultNewOptions(options: NewOptions) { @@ -139,6 +150,7 @@ export function applyDefaultNewOptions(options: NewOptions) { enableTypeDef: true, enableGithubActions: true, dryRun: false, + esm: false, ...options, } } diff --git a/cli/src/utils/__tests__/__snapshots__/version.spec.ts.md b/cli/src/utils/__tests__/__snapshots__/version.spec.ts.md index 23bf153b..e6972c74 100644 --- a/cli/src/utils/__tests__/__snapshots__/version.spec.ts.md +++ b/cli/src/utils/__tests__/__snapshots__/version.spec.ts.md @@ -10,11 +10,11 @@ Generated by [AVA](https://avajs.dev). [ '>= 8.6.0', - '>= 8.10.0 && < 9 || >= 9.3.0', - '>= 6.14.2 && < 7 || >= 8.11.2 && < 9 || >= 9.11.0', - '>= 10.16.0 && < 11 || >= 11.8.0', - '>= 10.17.0 && < 11 || >= 12.11.0', - '>= 10.20.0 && < 11 || >= 12.17.0 && < 13 || >= 14.0.0', - '>= 10.23.0 && < 11 || >= 12.19.0 && < 13 || >= 14.12.0', - '>= 12.22.0 && < 13 || >= 14.17.0 && < 15 || >= 15.12.0', + '>= 8.10.0 < 9 || >= 9.3.0', + '>= 6.14.2 < 7 || >= 8.11.2 < 9 || >= 9.11.0', + '>= 10.16.0 < 11 || >= 11.8.0', + '>= 10.17.0 < 11 || >= 12.11.0', + '>= 10.20.0 < 11 || >= 12.17.0 < 13 || >= 14.0.0', + '>= 10.23.0 < 11 || >= 12.19.0 < 13 || >= 14.12.0', + '>= 12.22.0 < 13 || >= 14.17.0 < 15 || >= 15.12.0', ] diff --git a/cli/src/utils/__tests__/__snapshots__/version.spec.ts.snap b/cli/src/utils/__tests__/__snapshots__/version.spec.ts.snap index 8e7feceb5ef11294941f2b33697fd4102fb1760e..657b24b23f470e4257e1a6ca9d30952530f1e061 100644 GIT binary patch literal 364 zcmV-y0h9hgRzVp1w4vp@c@FKai*bW;4$I7-+Pnw{as$hkJaj3>P+YIWmr9wi#(yJEToFG zq;aW~j5RHyr;N%X%?e4Cd|qTq&Sjyq3IpuuYPC+1NJqEu<&YlvJkL2kHR218}dDie3Tim*;r|_Hcv1J`x>r2o3>v0R77gI%5;YO#on%qdF0s zae7Wex?EB;LKZY=$D*g0OqdukAJM5nu`6n$>7ubIJGOj`5xycO^+Q4!8rfMx?^yc8 z(9?gkzcub!eE5g^yp5ZMz*-~5n}Mz!;AS9b4}`4&bT}GiKoo}Ofb?QaNY$l@v+8nWhC)lcB<@_fcZo0}5kZG( zqDeRP9ejZLI6i@oANQ$y2@jk~-Igd^@Wjt9g;qOqNo`TGFIa zN+z0?@k>r+ndPOVO1{o>C7)%f^BM!(=z6(I(^$uM)}c-MMkstyKIDV7^?}C9sbS0teWz$T8RioDmGJF6oSg%xeMwlboy*?lF&! z$Mk|mbg`i5Ma*y8mT7ZLLdGXdY}(VNjRbDImVn+CVH?CLMtF{g<|60K0PC}MuxAFe z8BG5V#LfxFEP~BKc%6%u;~RIvycW|ln6Sb4otUT#gTa>5-{A-*MCYRYoxr%kn)3^X K+`?dI0ssI;Jg>a~ diff --git a/cli/src/utils/__tests__/typegen.spec.ts b/cli/src/utils/__tests__/typegen.spec.ts index 0b6516a7..f00d8ebe 100644 --- a/cli/src/utils/__tests__/typegen.spec.ts +++ b/cli/src/utils/__tests__/typegen.spec.ts @@ -36,7 +36,7 @@ test('should ident string correctly', (t) => { }) test('should process type def correctly', async (t) => { - const dts = await processTypeDef( + const { dts } = await processTypeDef( join( fileURLToPath(import.meta.url), '../', diff --git a/cli/src/utils/config.ts b/cli/src/utils/config.ts index 2ad2f02e..e42a5b29 100644 --- a/cli/src/utils/config.ts +++ b/cli/src/utils/config.ts @@ -49,7 +49,7 @@ interface UserNapiConfig { } } -interface CommonPackageJsonFields { +export interface CommonPackageJsonFields { name: string version: string description?: string @@ -64,6 +64,17 @@ interface CommonPackageJsonFields { bugs?: any // eslint-disable-next-line no-use-before-define napi?: UserNapiConfig + type?: 'module' | 'commonjs' + scripts?: Record + + // modules + main?: string + module?: string + types?: string + exports?: any + + dependencies?: Record + devDependencies?: Record } export type NapiConfig = Required< diff --git a/cli/src/utils/typegen.ts b/cli/src/utils/typegen.ts index a42ed318..ad9cb003 100644 --- a/cli/src/utils/typegen.ts +++ b/cli/src/utils/typegen.ts @@ -54,6 +54,7 @@ export async function processTypeDef( intermediateTypeFile: string, header?: string, ) { + const exports: string[] = [] const defs = await readIntermediateTypeFile(intermediateTypeFile) const groupedDefs = preprocessTypeDef(defs) @@ -65,8 +66,23 @@ export async function processTypeDef( if (namespace === TOP_LEVEL_NAMESPACE) { for (const def of defs) { dts += prettyPrint(def, 0) + '\n\n' + switch (def.kind) { + case TypeDefKind.Const: + case TypeDefKind.Enum: + case TypeDefKind.Fn: + case TypeDefKind.Struct: { + exports.push(def.name) + if (def.original_name && def.original_name !== def.name) { + exports.push(def.original_name) + } + break + } + default: + break + } } } else { + exports.push(namespace) dts += `export namespace ${namespace} {\n` for (const def of defs) { dts += prettyPrint(def, 2) + '\n' @@ -87,7 +103,10 @@ export class ExternalObject { ` } - return header + dts + return { + dts: header + dts, + exports, + } } async function readIntermediateTypeFile(file: string) { diff --git a/cli/src/utils/version.ts b/cli/src/utils/version.ts index b5226f95..5470a257 100644 --- a/cli/src/utils/version.ts +++ b/cli/src/utils/version.ts @@ -68,7 +68,7 @@ function toEngineRequirement(versions: NodeVersion[]): string { requirements.push(req) }) - return requirements.join(' && ') + return requirements.join(' ') } export function napiEngineRequirement(napiVersion: NapiVersion): string {