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.

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

Generator emits cjs #1138

Merged
merged 4 commits into from
Jan 29, 2025
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
5 changes: 5 additions & 0 deletions .changeset/quiet-lions-sort.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@osdk/foundry-sdk-generator": minor
---

We now generate osdk libs that support commonjs
20 changes: 18 additions & 2 deletions packages/e2e.test.foundry-sdk-generator/generateMockOntology.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
// @ts-check
import { __testSeamOnly_NotSemverStable__GeneratePackageCommand as GeneratePackageCommand } from "@osdk/foundry-sdk-generator";
import { apiServer } from "@osdk/shared.test";
import { $ } from "execa";
import * as fs from "node:fs/promises";
import { tmpdir } from "node:os";
import * as path from "node:path";
Expand Down Expand Up @@ -98,13 +99,28 @@ async function setup() {
await safeStat(testApp2Dir, "should exist");
await safeStat(testApp2BetaDir, "should exist");

await $({
stdout: "inherit",
stderr: "inherit",
})`attw --pack ${
path.join(testApp2Dir, "osdk")
} --ignore-rules internal-resolution-error`;

await $({
stdout: "inherit",
stderr: "inherit",
})`attw --pack ${
path.join(testApp2BetaDir, "osdk")
} --ignore-rules internal-resolution-error`;

const finalOutDir = path.join(
path.dirname(fileURLToPath(import.meta.url)),
"src",
"generatedNoCheck",
);

fs.cp(dir, finalOutDir, { recursive: true });
await fs.rm(finalOutDir, { recursive: true, force: true });
await fs.cp(dir, finalOutDir, { recursive: true });
}

export async function teardown() {
Expand All @@ -121,7 +137,7 @@ await teardown();
*/
async function rmRf(testAppDir) {
try {
await fs.rm(testAppDir, { recursive: true });
await fs.rm(testAppDir, { recursive: true, force: true });
} catch (e) {
// console.debug("rm error", e);
// Only needed for regenerations
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ import {
createProgram,
createSourceFile,
ModuleKind,
ModuleResolutionKind,
ScriptTarget,
} from "typescript";

Expand All @@ -31,6 +30,7 @@ export interface CompilerOutput {

export function compileInMemory(
files: { [fileName: string]: string },
type: "cjs" | "esm",
): {
files: {
[fileName: string]: string;
Expand All @@ -39,9 +39,9 @@ export function compileInMemory(
} {
const inMemoryOutputFileSystem: { [fileName: string]: string } = {};
const compilerOptions: CompilerOptions = {
module: ModuleKind.NodeNext,
module: type === "cjs" ? ModuleKind.CommonJS : ModuleKind.ES2022,
target: ScriptTarget.ES2020,
moduleResolution: ModuleResolutionKind.NodeNext,
resolvePackageJsonExports: true,
Copy link
Contributor

Choose a reason for hiding this comment

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

So you have to explicitly set this now because you're not specifying moduleResolution anymore?

More concretely, if module is CommonJS, then moduleResolution becomes node10, in which case, it wont look at exports by default. But if its ES2022, then technically you don't need to explicitly set that line right cuz it'll default on?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. So I want to be sure we are generating in a way that will work with Node16 so I turn this on.

declaration: true,
skipLibCheck: true,
};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ export async function generatePackage(
},
): Promise<void> {
const { consola } = await import("consola");
let success = true;

const packagePath = join(options.outputDir, options.packageName);

Expand Down Expand Up @@ -82,16 +83,47 @@ export async function generatePackage(
beta: options.beta,
});

// writes to in memory fs that the compiler will read from
await hostFs.writeFile(
join(packagePath, "package.json"),
JSON.stringify(contents),
);
// eslint-disable-next-line @typescript-eslint/no-unsafe-assignment
const compilerOutput: Record<
"esm" | "cjs",
ReturnType<typeof compileInMemory>
> = {} as any;

for (const type of ["esm", "cjs"] as const) {
// writes to in memory fs that the compiler will read from
await hostFs.writeFile(
join(packagePath, "package.json"),
JSON.stringify({
...contents,
type: type === "cjs" ? "commonjs" : "module",
}),
);

const compilerOutput = compileInMemory(inMemoryFileSystem);
compilerOutput.diagnostics.forEach(d =>
consola.error(`Error compiling file`, d.file?.fileName, d.messageText)
);
compilerOutput[type] = compileInMemory(inMemoryFileSystem, type);
compilerOutput[type].diagnostics.forEach(d => {
consola.error(`Error compiling file`, d.file?.fileName, d.messageText);
success = false;
});

await mkdir(join(packagePath, "dist", "bundle"), { recursive: true });

await mkdir(join(packagePath, "esm"), { recursive: true });
await mkdir(join(packagePath, "cjs"), { recursive: true });

for (const [path, contents] of Object.entries(compilerOutput[type].files)) {
const newPath = path.replace(
packagePath,
join(packagePath, type),
);
await mkdir(dirname(newPath), { recursive: true });
await writeFile(newPath, contents, { flag: "w" });
}

void await writeFile(
join(packagePath, type, "package.json"),
JSON.stringify({ type: type === "esm" ? "module" : "commonjs" }),
);
}

await mkdir(join(packagePath, "dist", "bundle"), { recursive: true });

Expand All @@ -107,28 +139,24 @@ export async function generatePackage(
bundleDts = await bundleDependencies(
[],
options.packageName,
compilerOutput.files,
compilerOutput["esm"].files,
undefined,
);
} catch (e) {
consola.error("Failed bundling DTS", e);
success = false;
}
} else {
consola.error(
"Could not find node_modules directory, skipping DTS bundling",
);
success = false;
}

await Promise.all([
...Object.entries(compilerOutput.files).map(async ([path, contents]) => {
await writeFile(path, contents, { flag: "w" });
}),
await writeFile(
join(packagePath, "dist", "bundle", "index.d.ts"),
bundleDts,
{ flag: "w" },
),
]);
await writeFile(
join(packagePath, "dist", "bundle", "index.d.mts"),
bundleDts,
{ flag: "w" },
);

const absolutePackagePath = isAbsolute(options.outputDir)
? options.outputDir
Expand All @@ -138,5 +166,10 @@ export async function generatePackage(
await generateBundles(absolutePackagePath, options.packageName);
} catch (e) {
consola.error(e);
success = false;
}

if (!success) {
throw new Error("Failed to generate package");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -42,21 +42,29 @@ export async function generatePackageJson(options: {
const packageJson = {
name: options.packageName,
version: options.packageVersion,
main: "./index.js",
types: "./index.d.ts",
main: "./cjs/index.js",
Copy link
Contributor

Choose a reason for hiding this comment

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

So are these two fields cjs now because node10 default looks at the main field?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes

types: "./cjs/index.d.ts",
exports: {
".": {
types: "./index.d.ts",
script: {
types: "./dist/bundle/index.d.ts",
default: "./dist/bundle/index.esm.js",
types: "./dist/bundle/index.d.mts",
default: "./dist/bundle/index.mjs",
},
default: "./index.js",
require: {
types: "./cjs/index.d.ts",
default: "./cjs/index.js",
},
import: {
types: "./esm/index.d.ts",
default: "./esm/index.js",
},
types: "./cjs/index.d.ts",
default: "./cjs/index.js",
},
},
dependencies: packageDeps,
peerDependencies: packagePeerDeps,
type: "module",
type: "commonjs",
};

await writeFile(
Expand Down
20 changes: 13 additions & 7 deletions packages/foundry-sdk-generator/src/generate/generateBundles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,16 @@ import nodePolyfill from "rollup-plugin-polyfill-node";
async function createRollupBuild(
absolutePackagePath: string,
packageName: string,
) {
const inputPath = `${absolutePackagePath}/${packageName}/index.js`;
): Promise<RollupBuild> {
const inputPath = `${absolutePackagePath}/${packageName}/esm/index.js`;

const { findUp } = await import("find-up");
const nodeModulesPath = await findUp("node_modules", {
cwd: __dirname,
type: "directory",
});

return rollup({
return await rollup({
input: inputPath,
plugins: [
nodeResolve({
Expand Down Expand Up @@ -64,8 +64,9 @@ async function writeRollupBuild(
packageName: string,
format: ModuleFormat,
) {
const outputPath =
`${absolutePackagePath}/${packageName}/dist/bundle/index.${format}.js`;
const outputPath = `${absolutePackagePath}/${packageName}/dist/bundle/index.${
format === "cjs" ? "cjs" : "mjs"
}`;

await Promise.all([
rollupBuild.write({
Expand All @@ -84,13 +85,18 @@ async function generateEsmBuild(
absolutePackagePath: string,
packageName: string,
) {
const umdBuild = await createRollupBuild(absolutePackagePath, packageName);
const umdBuild = await createRollupBuild(
absolutePackagePath,
packageName,
);
await writeRollupBuild(umdBuild, absolutePackagePath, packageName, "esm");
}

export async function generateBundles(
absolutePackagePath: string,
packageName: string,
): Promise<void> {
await Promise.all([generateEsmBuild(absolutePackagePath, packageName)]);
await Promise.all([
generateEsmBuild(absolutePackagePath, packageName),
]);
}
Loading