Skip to content

Commit

Permalink
fix(cli): incorrect behaviors (#1626)
Browse files Browse the repository at this point in the history
* 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>
  • Loading branch information
forehalo and Brooooooklyn committed Jun 17, 2023
1 parent b34cca4 commit fb22a5a
Show file tree
Hide file tree
Showing 14 changed files with 154 additions and 55 deletions.
6 changes: 6 additions & 0 deletions cli/codegen/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
},
],
}

Expand Down
1 change: 1 addition & 0 deletions cli/docs/new.md
Original file line number Diff line number Diff line change
Expand Up @@ -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 |
31 changes: 21 additions & 10 deletions cli/src/api/build.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand All @@ -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,
Expand Down
1 change: 1 addition & 0 deletions cli/src/api/new.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ function generatePackageJson(options: NewOptions): Output {
license: options.license,
engineRequirement: napiEngineRequirement(options.minNodeApiVersion),
cliVersion: CLI_VERSION,
esm: options.esm,
}),
}
}
Expand Down
11 changes: 8 additions & 3 deletions cli/src/api/templates/js-binding.ts
Original file line number Diff line number Diff line change
@@ -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')
Expand Down Expand Up @@ -253,5 +255,8 @@ if (!nativeBinding) {
throw new Error(\`Failed to load native binding\`)
}
${idents
.map((ident) => `module.exports.${ident} = binding.${ident}`)
.join('\n')}
`
}
77 changes: 51 additions & 26 deletions cli/src/api/templates/package.json.ts
Original file line number Diff line number Diff line change
@@ -1,43 +1,68 @@
import { CommonPackageJsonFields } from '../../utils/config.js'

export const createPackageJson = ({
name,
binaryName,
targets,
license,
engineRequirement,
cliVersion,
esm,
}: {
name: string
binaryName: string
targets: string[]
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)
}
18 changes: 13 additions & 5 deletions cli/src/commands/new.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand Down Expand Up @@ -94,10 +95,6 @@ export class NewCommand extends BaseNewCommand {
}

private async fetchTargets(): Promise<TargetTriple[]> {
if (this.enableDefaultTargets) {
return DEFAULT_TARGETS.concat()
}

if (this.enableAllTargets) {
return AVAILABLE_TARGETS.concat()
}
Expand All @@ -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,
})

Expand Down Expand Up @@ -135,4 +132,15 @@ export class NewCommand extends BaseNewCommand {

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
}
}
12 changes: 12 additions & 0 deletions cli/src/def/new.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -63,6 +67,7 @@ export abstract class BaseNewCommand extends Command {
enableTypeDef: this.enableTypeDef,
enableGithubActions: this.enableGithubActions,
dryRun: this.dryRun,
esm: this.esm,
}
}
}
Expand Down Expand Up @@ -127,6 +132,12 @@ export interface NewOptions {
* @default false
*/
dryRun?: boolean
/**
* Whether enable ESM support
*
* @default false
*/
esm?: boolean
}

export function applyDefaultNewOptions(options: NewOptions) {
Expand All @@ -139,6 +150,7 @@ export function applyDefaultNewOptions(options: NewOptions) {
enableTypeDef: true,
enableGithubActions: true,
dryRun: false,
esm: false,
...options,
}
}
14 changes: 7 additions & 7 deletions cli/src/utils/__tests__/__snapshots__/version.spec.ts.md
Original file line number Diff line number Diff line change
Expand Up @@ -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',
]
Binary file modified cli/src/utils/__tests__/__snapshots__/version.spec.ts.snap
Binary file not shown.
2 changes: 1 addition & 1 deletion cli/src/utils/__tests__/typegen.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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),
'../',
Expand Down
13 changes: 12 additions & 1 deletion cli/src/utils/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ interface UserNapiConfig {
}
}

interface CommonPackageJsonFields {
export interface CommonPackageJsonFields {
name: string
version: string
description?: string
Expand All @@ -64,6 +64,17 @@ interface CommonPackageJsonFields {
bugs?: any
// eslint-disable-next-line no-use-before-define
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<
Expand Down
21 changes: 20 additions & 1 deletion cli/src/utils/typegen.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ export async function processTypeDef(
intermediateTypeFile: string,
header?: string,
) {
const exports: string[] = []
const defs = await readIntermediateTypeFile(intermediateTypeFile)
const groupedDefs = preprocessTypeDef(defs)

Expand All @@ -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'
Expand All @@ -87,7 +103,10 @@ export class ExternalObject<T> {
`
}

return header + dts
return {
dts: header + dts,
exports,
}
}

async function readIntermediateTypeFile(file: string) {
Expand Down
Loading

0 comments on commit fb22a5a

Please sign in to comment.