Skip to content
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.

Sign up for GitHub

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

feat(api): proper esm support and codegen overhauls #754

Merged
merged 23 commits into from
Oct 16, 2023
Merged

feat(api): proper esm support and codegen overhauls #754

Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
8e5558d
feat(api): wip of proper esm support and codegen overhauls
erunion Oct 11, 2023
0db62a3
Merge branch 'main' into refactor/codegen
erunion Oct 11, 2023
84872e6
docs: documenting why we're exporting `createSDK` the way we are
erunion Oct 12, 2023
e2e1259
chore(deps): bumping ts-morph
erunion Oct 12, 2023
6463a02
fix: overhauling schemas to now be stored in a `schemas/` directory
erunion Oct 12, 2023
ccbdc40
Merge branch 'main' into refactor/codegen
erunion Oct 12, 2023
654194e
fix: storage system didn't properly create subdirectories
erunion Oct 13, 2023
9ade11b
fix: broken test
erunion Oct 13, 2023
8e11923
fix: flaky test in ci
erunion Oct 13, 2023
d33e655
fix: bringing back some tests
erunion Oct 13, 2023
7d7b4cc
feat: refactoring codegen'd sdks to be placed into a `src/` directory
erunion Oct 13, 2023
d4e5d5a
fix: pinning installed sdk dependencies to known working versions
erunion Oct 13, 2023
330a3a1
fix: cjs compatibility with our `types` file being exported in `index`
erunion Oct 16, 2023
488041e
fix: removing some dead code
erunion Oct 16, 2023
9fd0370
fix: compatibility with node 20 on `npm run debug:bin`
erunion Oct 16, 2023
b1e705d
v7.0.0-alpha.5
erunion Oct 16, 2023
87f7319
fix: minor `tsup` codegen fixes (#759)
kanadgupta Oct 16, 2023
594710c
Update packages/api/src/codegen/languages/typescript/index.ts
erunion Oct 16, 2023
c7c7f55
Revert "refactor: slight cleanup"
kanadgupta Oct 16, 2023
e681ee7
Update packages/api/test/codegen/languages/typescript/index.test.ts
erunion Oct 16, 2023
6a66d22
Update packages/api/test/codegen/languages/typescript/index.test.ts
erunion Oct 16, 2023
7173c80
fix: pr feedback
erunion Oct 16, 2023
d591696
fix: ts ignore typo
erunion Oct 16, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
185 changes: 77 additions & 108 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions packages/api/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
"scripts": {
"attw": "attw --pack --format table-flipped",
"build": "tsc",
"debug:bin": "node -r ts-node/register src/bin.ts",
"debug:bin": "ts-node --esm src/bin.ts",
erunion marked this conversation as resolved.
Show resolved Hide resolved
"lint:types": "tsc --noEmit",
"prebuild": "rm -rf dist/; npm run version",
"prepack": "npm run build",
Expand Down Expand Up @@ -54,7 +54,8 @@
"prompts": "^2.4.2",
"semver": "^7.3.8",
"ssri": "^10.0.1",
"ts-morph": "^17.0.1",
"ts-morph": "^20.0.0",
"tsup": "^7.2.0",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we remove this as a dep since it's being installed in the codegen'd directory now?

"validate-npm-package-name": "^5.0.0"
},
"devDependencies": {
Expand Down
13 changes: 11 additions & 2 deletions packages/api/src/codegen/codegenerator.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,14 @@ export default abstract class CodeGenerator {

userAgent: string;

requiredPackages!: Record<string, { reason: string; url: string }>;
requiredPackages!: Record<
string,
{
reason: string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as a follow-up to the work I added in #759, i was thinking it might make sense to have a devDep flag that splits out our required deps (e.g., @readme/api-core) from our required dev-deps (e.g., typescript)

Suggested change
reason: string;
devDependency: boolean;
reason: string;

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

will do this in another PR since it's going to require a little bit of work

url: string;
version: string;
}
>;

constructor(spec: Oas, specPath: string, identifier: string) {
this.spec = spec;
Expand Down Expand Up @@ -55,10 +62,12 @@ export default abstract class CodeGenerator {
}
}

abstract compile(): Promise<Record<string, string>>;
abstract generate(): Promise<Record<string, string>>;

abstract install(storage: Storage, opts?: InstallerOptions): Promise<void>;

abstract compile(storage: Storage, opts?: InstallerOptions): Promise<void>;

hasRequiredPackages() {
return Boolean(Object.keys(this.requiredPackages));
}
Expand Down
18 changes: 5 additions & 13 deletions packages/api/src/codegen/factory.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import type Oas from 'oas';

import TSGenerator from './languages/typescript/index.js';

export type SupportedLanguages = 'js' | 'js-cjs' | 'js-esm' | 'ts';
export enum SupportedLanguages {
kanadgupta marked this conversation as resolved.
Show resolved Hide resolved
JS = 'js',
}

export default function codegenFactory(
language: SupportedLanguages,
Expand All @@ -12,18 +14,8 @@ export default function codegenFactory(
identifier: string,
): CodeGenerator {
switch (language) {
case 'js':
throw new TypeError('An export format of CommonJS or ECMAScript is required for JavaScript compilation.');

case 'js-cjs':
case 'js-esm':
case 'ts':
return new TSGenerator(spec, specPath, identifier, {
outputJS: ['js-cjs', 'js-esm'].includes(language),

// TS will always generate with ESM-like exports.
compilerTarget: language === 'js-cjs' ? 'cjs' : 'esm',
});
case SupportedLanguages.JS:
return new TSGenerator(spec, specPath, identifier);

default:
throw new TypeError(`Unsupported language supplied: ${language}`);
Expand Down
Loading