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 <lynweklm@gmail.com>
This commit is contained in:
liuyi 2023-06-17 12:49:30 +08:00 committed by GitHub
parent b34cca4d83
commit fb22a5ae07
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
14 changed files with 154 additions and 55 deletions

View file

@ -103,6 +103,12 @@ const NEW_OPTIONS: CommandSchema = {
description: 'Whether to run the command in dry-run mode', description: 'Whether to run the command in dry-run mode',
default: false, default: false,
}, },
{
name: 'esm',
type: 'boolean',
description: 'Whether enable ESM support',
default: false,
},
], ],
} }

View file

@ -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 | | 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 | | 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 | | dryRun | --dry-run | boolean | false | false | Whether to run the command in dry-run mode |
| esm | --esm | boolean | false | false | Whether enable ESM support |

View file

@ -28,7 +28,7 @@ import {
writeFileAsync, writeFileAsync,
} from '../utils/index.js' } from '../utils/index.js'
import { createJsBinding } from './templates/index.js' import { createCjsBinding } from './templates/index.js'
const debug = debugFactory('build') const debug = debugFactory('build')
@ -452,8 +452,8 @@ class Builder {
// only for cdylib // only for cdylib
if (this.cdyLibName) { if (this.cdyLibName) {
await this.generateTypeDef() const idents = await this.generateTypeDef()
await this.writeJsBinding() await this.writeJsBinding(idents)
} }
return this.outputs return this.outputs
@ -523,12 +523,12 @@ class Builder {
private async generateTypeDef() { private async generateTypeDef() {
if (!(await fileExists(this.envs.TYPE_DEF_TMP_PATH))) { if (!(await fileExists(this.envs.TYPE_DEF_TMP_PATH))) {
return return []
} }
const dest = join(this.outputDir, this.options.dts ?? 'index.d.ts') 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.envs.TYPE_DEF_TMP_PATH,
!this.options.noDtsHeader !this.options.noDtsHeader
? this.options.dtsHeader ?? DEFAULT_TYPE_DEF_HEADER ? this.options.dtsHeader ?? DEFAULT_TYPE_DEF_HEADER
@ -547,21 +547,32 @@ class Builder {
debug.error('Failed to write type def file') debug.error('Failed to write type def file')
debug.error(e as Error) debug.error(e as Error)
} }
return exports
} }
private async writeJsBinding() { private async writeJsBinding(idents: string[]) {
if (!this.options.platform || this.options.noJsBinding) { if (
!this.options.platform ||
this.options.noJsBinding ||
idents.length === 0
) {
return 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 { try {
const dest = join(this.outputDir, `${name}.js`)
debug('Writing js binding to:') debug('Writing js binding to:')
debug(' %i', dest) debug(' %i', dest)
await writeFileAsync(dest, js, 'utf-8') await writeFileAsync(dest, cjs, 'utf-8')
this.outputs.push({ this.outputs.push({
kind: 'js', kind: 'js',
path: dest, path: dest,

View file

@ -146,6 +146,7 @@ function generatePackageJson(options: NewOptions): Output {
license: options.license, license: options.license,
engineRequirement: napiEngineRequirement(options.minNodeApiVersion), engineRequirement: napiEngineRequirement(options.minNodeApiVersion),
cliVersion: CLI_VERSION, cliVersion: CLI_VERSION,
esm: options.esm,
}), }),
} }
} }

View file

@ -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 return `// prettier-ignore
/* tslint:disable */
/* eslint-disable */ /* eslint-disable */
/* auto-generated by NAPI-RS */ /* auto-generated by NAPI-RS */
const { existsSync, readFileSync } = require('fs') const { existsSync, readFileSync } = require('fs')
@ -253,5 +255,8 @@ if (!nativeBinding) {
throw new Error(\`Failed to load native binding\`) throw new Error(\`Failed to load native binding\`)
} }
${idents
.map((ident) => `module.exports.${ident} = binding.${ident}`)
.join('\n')}
` `
} }

View file

@ -1,3 +1,5 @@
import { CommonPackageJsonFields } from '../../utils/config.js'
export const createPackageJson = ({ export const createPackageJson = ({
name, name,
binaryName, binaryName,
@ -5,6 +7,7 @@ export const createPackageJson = ({
license, license,
engineRequirement, engineRequirement,
cliVersion, cliVersion,
esm,
}: { }: {
name: string name: string
binaryName: string binaryName: string
@ -12,32 +15,54 @@ export const createPackageJson = ({
license: string license: string
engineRequirement: string engineRequirement: string
cliVersion: string cliVersion: string
esm: boolean
}) => { }) => {
return `{ const content: CommonPackageJsonFields = {
"name": "${name}", name,
"version": "1.0.0", version: '1.0.0',
"main": "index.js", license,
"types": "index.d.ts", engines: {
"license": "${license}", node: engineRequirement,
"engines": { },
"node": "${engineRequirement}" type: 'commonjs',
}, main: 'index.js',
"napi": { types: 'index.d.ts',
"binaryName": "${binaryName}", module: undefined,
"targets": [ exports: undefined,
${targets.map((t) => `"${t}"`).join(',\n ')} napi: {
] binaryName,
}, targets,
"scripts": { },
"test": "node -e \\"assert(require('.').sum(1, 2) === 3)\\"", scripts: {
"build": "napi build --release --platform --strip", test: 'node -e "assert(require(\'.\').sum(1, 2) === 3)"',
"build:debug": "napi build", build: 'napi build --release --platform --strip',
"prepublishOnly": "napi prepublish -t npm", 'build:debug': 'napi build',
"artifacts": "napi artifacts", prepublishOnly: 'napi prepublish -t npm',
"version": "napi version" artifacts: 'napi artifacts',
}, version: 'napi version',
"devDependencies": { },
"@napi-rs/cli": "^${cliVersion}" 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)
} }

View file

@ -40,6 +40,7 @@ export class NewCommand extends BaseNewCommand {
return { return {
...cmdOptions, ...cmdOptions,
name: await this.fetchName(path.parse(cmdOptions.path).base), name: await this.fetchName(path.parse(cmdOptions.path).base),
esm: await this.fetchEnableEsm(),
minNodeApiVersion: await this.fetchNapiVersion(), minNodeApiVersion: await this.fetchNapiVersion(),
targets: await this.fetchTargets(), targets: await this.fetchTargets(),
license: await this.fetchLicense(), license: await this.fetchLicense(),
@ -94,10 +95,6 @@ export class NewCommand extends BaseNewCommand {
} }
private async fetchTargets(): Promise<TargetTriple[]> { private async fetchTargets(): Promise<TargetTriple[]> {
if (this.enableDefaultTargets) {
return DEFAULT_TARGETS.concat()
}
if (this.enableAllTargets) { if (this.enableAllTargets) {
return AVAILABLE_TARGETS.concat() return AVAILABLE_TARGETS.concat()
} }
@ -107,7 +104,7 @@ export class NewCommand extends BaseNewCommand {
type: 'checkbox', type: 'checkbox',
loop: false, loop: false,
message: 'Choose target(s) your crate will be compiled to', message: 'Choose target(s) your crate will be compiled to',
default: DEFAULT_TARGETS, default: this.enableDefaultTargets ? DEFAULT_TARGETS : [],
choices: AVAILABLE_TARGETS, choices: AVAILABLE_TARGETS,
}) })
@ -135,4 +132,15 @@ export class NewCommand extends BaseNewCommand {
return enableGithubActions return enableGithubActions
} }
private async fetchEnableEsm(): Promise<boolean> {
const { esm } = await inquirer.prompt({
name: 'esm',
type: 'confirm',
message: 'Enable ESM support',
default: this.esm,
})
return esm
}
} }

View file

@ -51,6 +51,10 @@ export abstract class BaseNewCommand extends Command {
description: 'Whether to run the command in dry-run mode', description: 'Whether to run the command in dry-run mode',
}) })
esm = Option.Boolean('--esm', false, {
description: 'Whether enable ESM support',
})
getOptions() { getOptions() {
return { return {
path: this.$$path, path: this.$$path,
@ -63,6 +67,7 @@ export abstract class BaseNewCommand extends Command {
enableTypeDef: this.enableTypeDef, enableTypeDef: this.enableTypeDef,
enableGithubActions: this.enableGithubActions, enableGithubActions: this.enableGithubActions,
dryRun: this.dryRun, dryRun: this.dryRun,
esm: this.esm,
} }
} }
} }
@ -127,6 +132,12 @@ export interface NewOptions {
* @default false * @default false
*/ */
dryRun?: boolean dryRun?: boolean
/**
* Whether enable ESM support
*
* @default false
*/
esm?: boolean
} }
export function applyDefaultNewOptions(options: NewOptions) { export function applyDefaultNewOptions(options: NewOptions) {
@ -139,6 +150,7 @@ export function applyDefaultNewOptions(options: NewOptions) {
enableTypeDef: true, enableTypeDef: true,
enableGithubActions: true, enableGithubActions: true,
dryRun: false, dryRun: false,
esm: false,
...options, ...options,
} }
} }

View file

@ -10,11 +10,11 @@ Generated by [AVA](https://avajs.dev).
[ [
'>= 8.6.0', '>= 8.6.0',
'>= 8.10.0 && < 9 || >= 9.3.0', '>= 8.10.0 < 9 || >= 9.3.0',
'>= 6.14.2 && < 7 || >= 8.11.2 && < 9 || >= 9.11.0', '>= 6.14.2 < 7 || >= 8.11.2 < 9 || >= 9.11.0',
'>= 10.16.0 && < 11 || >= 11.8.0', '>= 10.16.0 < 11 || >= 11.8.0',
'>= 10.17.0 && < 11 || >= 12.11.0', '>= 10.17.0 < 11 || >= 12.11.0',
'>= 10.20.0 && < 11 || >= 12.17.0 && < 13 || >= 14.0.0', '>= 10.20.0 < 11 || >= 12.17.0 < 13 || >= 14.0.0',
'>= 10.23.0 && < 11 || >= 12.19.0 && < 13 || >= 14.12.0', '>= 10.23.0 < 11 || >= 12.19.0 < 13 || >= 14.12.0',
'>= 12.22.0 && < 13 || >= 14.17.0 && < 15 || >= 15.12.0', '>= 12.22.0 < 13 || >= 14.17.0 < 15 || >= 15.12.0',
] ]

View file

@ -36,7 +36,7 @@ test('should ident string correctly', (t) => {
}) })
test('should process type def correctly', async (t) => { test('should process type def correctly', async (t) => {
const dts = await processTypeDef( const { dts } = await processTypeDef(
join( join(
fileURLToPath(import.meta.url), fileURLToPath(import.meta.url),
'../', '../',

View file

@ -49,7 +49,7 @@ interface UserNapiConfig {
} }
} }
interface CommonPackageJsonFields { export interface CommonPackageJsonFields {
name: string name: string
version: string version: string
description?: string description?: string
@ -64,6 +64,17 @@ interface CommonPackageJsonFields {
bugs?: any bugs?: any
// eslint-disable-next-line no-use-before-define // eslint-disable-next-line no-use-before-define
napi?: UserNapiConfig napi?: UserNapiConfig
type?: 'module' | 'commonjs'
scripts?: Record<string, string>
// modules
main?: string
module?: string
types?: string
exports?: any
dependencies?: Record<string, string>
devDependencies?: Record<string, string>
} }
export type NapiConfig = Required< export type NapiConfig = Required<

View file

@ -54,6 +54,7 @@ export async function processTypeDef(
intermediateTypeFile: string, intermediateTypeFile: string,
header?: string, header?: string,
) { ) {
const exports: string[] = []
const defs = await readIntermediateTypeFile(intermediateTypeFile) const defs = await readIntermediateTypeFile(intermediateTypeFile)
const groupedDefs = preprocessTypeDef(defs) const groupedDefs = preprocessTypeDef(defs)
@ -65,8 +66,23 @@ export async function processTypeDef(
if (namespace === TOP_LEVEL_NAMESPACE) { if (namespace === TOP_LEVEL_NAMESPACE) {
for (const def of defs) { for (const def of defs) {
dts += prettyPrint(def, 0) + '\n\n' 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 { } else {
exports.push(namespace)
dts += `export namespace ${namespace} {\n` dts += `export namespace ${namespace} {\n`
for (const def of defs) { for (const def of defs) {
dts += prettyPrint(def, 2) + '\n' dts += prettyPrint(def, 2) + '\n'
@ -87,7 +103,10 @@ export class ExternalObject<T> {
` `
} }
return header + dts return {
dts: header + dts,
exports,
}
} }
async function readIntermediateTypeFile(file: string) { async function readIntermediateTypeFile(file: string) {

View file

@ -68,7 +68,7 @@ function toEngineRequirement(versions: NodeVersion[]): string {
requirements.push(req) requirements.push(req)
}) })
return requirements.join(' && ') return requirements.join(' ')
} }
export function napiEngineRequirement(napiVersion: NapiVersion): string { export function napiEngineRequirement(napiVersion: NapiVersion): string {