From f15f245c3652bdcd172f3469d476443ca85b30d5 Mon Sep 17 00:00:00 2001 From: Momo Kornher Date: Wed, 5 Jul 2023 11:41:28 +0100 Subject: [PATCH] feat: export symbol-id as a submodule The `symbolIdentifier` function is a base jsii feature that other tools can depend on without having to depend on the rest of jsii. Currently a dependant package has to import all of the jsii code just to get `symbolIdentifier`. This can have performance implications, as well as producing larger bundles. This change exports `symbolIdentifier` as a submodule to allow a direct import. --- .projenrc.ts | 1 + package.json | 3 +- src/assembler.ts | 2 +- src/common/README.md | 8 ++++ src/common/find-utils.ts | 62 ++++++++++++++++++++++++++ src/common/index.ts | 1 + src/{ => common}/symbol-id.ts | 4 +- src/compiler.ts | 3 +- src/index.ts | 2 +- src/project-info.ts | 3 +- src/transforms/deprecation-warnings.ts | 2 +- src/utils.ts | 62 -------------------------- 12 files changed, 83 insertions(+), 70 deletions(-) create mode 100644 src/common/README.md create mode 100644 src/common/find-utils.ts create mode 100644 src/common/index.ts rename src/{ => common}/symbol-id.ts (98%) diff --git a/.projenrc.ts b/.projenrc.ts index f9dadf2d..4d83e504 100644 --- a/.projenrc.ts +++ b/.projenrc.ts @@ -150,6 +150,7 @@ project.package.addField('exports', { '.': `./${project.package.entrypoint}`, './bin/jsii': './bin/jsii', './package.json': './package.json', + './common': './lib/common/index.js', }); // Remove TypeScript devDependency (it's a direct/normal dependency here) diff --git a/package.json b/package.json index c2d511c1..26925321 100644 --- a/package.json +++ b/package.json @@ -93,7 +93,8 @@ "exports": { ".": "./lib/index.js", "./bin/jsii": "./bin/jsii", - "./package.json": "./package.json" + "./package.json": "./package.json", + "./common": "./lib/common/index.js" }, "//": "~~ Generated by projen. To modify, edit .projenrc.ts and run \"npx projen\"." } diff --git a/src/assembler.ts b/src/assembler.ts index 172f66a2..3a3b16e6 100644 --- a/src/assembler.ts +++ b/src/assembler.ts @@ -9,6 +9,7 @@ import * as log4js from 'log4js'; import * as ts from 'typescript'; import * as Case from './case'; +import { symbolIdentifier } from './common/symbol-id'; import { Directives } from './directives'; import { getReferencedDocParams, parseSymbolDocumentation, TypeSystemHints } from './docs'; import { Emitter } from './emitter'; @@ -17,7 +18,6 @@ import * as literate from './literate'; import * as bindings from './node-bindings'; import { ProjectInfo } from './project-info'; import { isReservedName } from './reserved-words'; -import { symbolIdentifier } from './symbol-id'; import { DeprecatedRemover } from './transforms/deprecated-remover'; import { DeprecationWarningsInjector } from './transforms/deprecation-warnings'; import { RuntimeTypeInfoInjector } from './transforms/runtime-info'; diff --git a/src/common/README.md b/src/common/README.md new file mode 100644 index 00000000..6fdbc372 --- /dev/null +++ b/src/common/README.md @@ -0,0 +1,8 @@ +# jsii/common + +jsii has some features that other packages might need to depend on, without needing the whole of jsii. + +This submodule is addressing this need by exporting *small, self-contained* functions.s + +Anything in here MUST NOT depend on any other code in jsii. +It SHOULD be kept very lightweight and mostly depend on TypeScript and Node built-ins. diff --git a/src/common/find-utils.ts b/src/common/find-utils.ts new file mode 100644 index 00000000..87a9b79c --- /dev/null +++ b/src/common/find-utils.ts @@ -0,0 +1,62 @@ +import * as fs from 'node:fs'; +import * as path from 'node:path'; + +/** + * Find a directory up the tree from a starting directory matching a condition + * + * Will return `undefined` if no directory matches + * + * (This code is duplicated among jsii/jsii-pacmak/jsii-reflect. Changes should be done in all + * 3 locations, and we should unify these at some point: https://github.com/aws/jsii/issues/3236) + */ +export function findUp(directory: string, pred: (dir: string) => boolean): string | undefined { + const result = pred(directory); + + if (result) { + return directory; + } + + const parent = path.dirname(directory); + if (parent === directory) { + return undefined; + } + + return findUp(parent, pred); +} + +/** + * Find the package.json for a given package upwards from the given directory + * + * (This code is duplicated among jsii/jsii-pacmak/jsii-reflect. Changes should be done in all + * 3 locations, and we should unify these at some point: https://github.com/aws/jsii/issues/3236) + */ +export function findPackageJsonUp(packageName: string, directory: string) { + return findUp(directory, (dir) => { + const pjFile = path.join(dir, 'package.json'); + return fs.existsSync(pjFile) && JSON.parse(fs.readFileSync(pjFile, 'utf-8')).name === packageName; + }); +} + +/** + * Find the directory that contains a given dependency, identified by its 'package.json', from a starting search directory + * + * (This code is duplicated among jsii/jsii-pacmak/jsii-reflect. Changes should be done in all + * 3 locations, and we should unify these at some point: https://github.com/aws/jsii/issues/3236) + */ +export function findDependencyDirectory(dependencyName: string, searchStart: string) { + // Explicitly do not use 'require("dep/package.json")' because that will fail if the + // package does not export that particular file. + const entryPoint = require.resolve(dependencyName, { + paths: [searchStart], + }); + + // Search up from the given directory, looking for a package.json that matches + // the dependency name (so we don't accidentally find stray 'package.jsons'). + const depPkgJsonPath = findPackageJsonUp(dependencyName, path.dirname(entryPoint)); + + if (!depPkgJsonPath) { + throw new Error(`Could not find dependency '${dependencyName}' from '${searchStart}'`); + } + + return depPkgJsonPath; +} diff --git a/src/common/index.ts b/src/common/index.ts new file mode 100644 index 00000000..628ac7f9 --- /dev/null +++ b/src/common/index.ts @@ -0,0 +1 @@ +export { symbolIdentifier } from './symbol-id'; diff --git a/src/symbol-id.ts b/src/common/symbol-id.ts similarity index 98% rename from src/symbol-id.ts rename to src/common/symbol-id.ts index f0767823..90209739 100644 --- a/src/symbol-id.ts +++ b/src/common/symbol-id.ts @@ -1,9 +1,9 @@ import * as fs from 'node:fs'; import * as path from 'node:path'; -import { Assembly } from '@jsii/spec'; +import type { Assembly } from '@jsii/spec'; import * as ts from 'typescript'; -import { findUp } from './utils'; +import { findUp } from './find-utils'; /** * Additional options that may be provided to the symbolIdentifier. diff --git a/src/compiler.ts b/src/compiler.ts index 30bc1cdd..e85793ad 100644 --- a/src/compiler.ts +++ b/src/compiler.ts @@ -6,6 +6,7 @@ import * as ts from 'typescript'; import { Assembler } from './assembler'; import * as Case from './case'; +import { findDependencyDirectory } from './common/find-utils'; import { emitDownleveledDeclarations, TYPES_COMPAT } from './downlevel-dts'; import { Emitter } from './emitter'; import { JsiiDiagnostic } from './jsii-diagnostic'; @@ -495,7 +496,7 @@ export class Compiler implements Emitter { } try { - const depDir = utils.findDependencyDirectory(depName, this.options.projectInfo.projectRoot); + const depDir = findDependencyDirectory(depName, this.options.projectInfo.projectRoot); const dep = path.join(depDir, 'tsconfig.json'); if (!fs.existsSync(dep)) { diff --git a/src/index.ts b/src/index.ts index 8f1fff9e..4a7d6f54 100644 --- a/src/index.ts +++ b/src/index.ts @@ -1,3 +1,3 @@ export * from './jsii-diagnostic'; -export * from './symbol-id'; +export * from './common/symbol-id'; export * from './helpers'; diff --git a/src/project-info.ts b/src/project-info.ts index 101056e9..92c90d54 100644 --- a/src/project-info.ts +++ b/src/project-info.ts @@ -6,8 +6,9 @@ import * as log4js from 'log4js'; import * as semver from 'semver'; import * as ts from 'typescript'; +import { findDependencyDirectory } from './common/find-utils'; import { JsiiDiagnostic } from './jsii-diagnostic'; -import { parsePerson, parseRepository, findDependencyDirectory } from './utils'; +import { parsePerson, parseRepository } from './utils'; // eslint-disable-next-line @typescript-eslint/no-var-requires, @typescript-eslint/no-require-imports const spdx: Set = require('spdx-license-list/simple'); diff --git a/src/transforms/deprecation-warnings.ts b/src/transforms/deprecation-warnings.ts index 0afa33d6..34fabaad 100644 --- a/src/transforms/deprecation-warnings.ts +++ b/src/transforms/deprecation-warnings.ts @@ -4,8 +4,8 @@ import * as spec from '@jsii/spec'; import { Assembly } from '@jsii/spec'; import * as ts from 'typescript'; +import { symbolIdentifier } from '../common/symbol-id'; import { ProjectInfo } from '../project-info'; -import { symbolIdentifier } from '../symbol-id'; export const WARNINGSCODE_FILE_NAME = '.warnings.jsii.js'; const WARNING_FUNCTION_NAME = 'print'; diff --git a/src/utils.ts b/src/utils.ts index a03cf7ee..767b030f 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -1,5 +1,3 @@ -import * as fs from 'node:fs'; -import * as path from 'node:path'; import * as log4js from 'log4js'; import * as ts from 'typescript'; @@ -153,66 +151,6 @@ export function parseRepository(value: string): { url: string } { } } -/** - * Find the directory that contains a given dependency, identified by its 'package.json', from a starting search directory - * - * (This code is duplicated among jsii/jsii-pacmak/jsii-reflect. Changes should be done in all - * 3 locations, and we should unify these at some point: https://github.com/aws/jsii/issues/3236) - */ -export function findDependencyDirectory(dependencyName: string, searchStart: string) { - // Explicitly do not use 'require("dep/package.json")' because that will fail if the - // package does not export that particular file. - const entryPoint = require.resolve(dependencyName, { - paths: [searchStart], - }); - - // Search up from the given directory, looking for a package.json that matches - // the dependency name (so we don't accidentally find stray 'package.jsons'). - const depPkgJsonPath = findPackageJsonUp(dependencyName, path.dirname(entryPoint)); - - if (!depPkgJsonPath) { - throw new Error(`Could not find dependency '${dependencyName}' from '${searchStart}'`); - } - - return depPkgJsonPath; -} - -/** - * Find the package.json for a given package upwards from the given directory - * - * (This code is duplicated among jsii/jsii-pacmak/jsii-reflect. Changes should be done in all - * 3 locations, and we should unify these at some point: https://github.com/aws/jsii/issues/3236) - */ -export function findPackageJsonUp(packageName: string, directory: string) { - return findUp(directory, (dir) => { - const pjFile = path.join(dir, 'package.json'); - return fs.existsSync(pjFile) && JSON.parse(fs.readFileSync(pjFile, 'utf-8')).name === packageName; - }); -} - -/** - * Find a directory up the tree from a starting directory matching a condition - * - * Will return `undefined` if no directory matches - * - * (This code is duplicated among jsii/jsii-pacmak/jsii-reflect. Changes should be done in all - * 3 locations, and we should unify these at some point: https://github.com/aws/jsii/issues/3236) - */ -export function findUp(directory: string, pred: (dir: string) => boolean): string | undefined { - const result = pred(directory); - - if (result) { - return directory; - } - - const parent = path.dirname(directory); - if (parent === directory) { - return undefined; - } - - return findUp(parent, pred); -} - const ANSI_REGEX = // eslint-disable-next-line no-control-regex /[\u001b\u009b][[()#;?]*(?:[0-9]{1,4}(?:;[0-9]{0,4})*)?[0-9A-ORZcf-nqry=><]/g;