Skip to content

Commit

Permalink
fix: TS input when targeting ESM with type: module (#297)
Browse files Browse the repository at this point in the history
This PR fixes a bug where TS input was not considering ESM with `type:
module` in package.json

Related to #129 which added package.json emission but only for JS input.
  • Loading branch information
styfle authored Nov 4, 2022
1 parent 66dc1ae commit 40a8710
Show file tree
Hide file tree
Showing 9 changed files with 83 additions and 47 deletions.
95 changes: 49 additions & 46 deletions src/node-file-trace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { NodeFileTraceOptions, NodeFileTraceResult, NodeFileTraceReasons, Stats,
import { basename, dirname, extname, relative, resolve, sep } from 'path';
import fs from 'graceful-fs';
import analyze, { AnalyzeResult } from './analyze';
import resolveDependency from './resolve-dependency';
import resolveDependency, { NotFoundError } from './resolve-dependency';
import { isMatch } from 'micromatch';
import { sharedLibEmit } from './utils/sharedlib-emit';
import { join } from 'path';
Expand Down Expand Up @@ -214,6 +214,45 @@ export class Job {
}
}

private maybeEmitDep = async (dep: string, path: string, cjsResolve: boolean) => {
let resolved: string | string[] = '';
let error: Error | undefined;
try {
resolved = await this.resolve(dep, path, this, cjsResolve);
} catch (e1: any) {
error = e1;
try {
if (this.ts && dep.endsWith('.js') && e1 instanceof NotFoundError) {
// TS with ESM relative import paths need full extensions
// (we have to write import "./foo.js" instead of import "./foo")
// See https://www.typescriptlang.org/docs/handbook/esm-node.html
const depTS = dep.slice(0, -3) + '.ts';
resolved = await this.resolve(depTS, path, this, cjsResolve);
error = undefined;
}
} catch (e2: any) {
error = e2;
}
}

if (error) {
this.warnings.add(new Error(`Failed to resolve dependency "${dep}":\n${error?.message}`));
return;
}

if (Array.isArray(resolved)) {
for (const item of resolved) {
// ignore builtins
if (item.startsWith('node:')) return;
await this.emitDependency(item, path);
}
} else {
// ignore builtins
if (resolved.startsWith('node:')) return;
await this.emitDependency(resolved, path);
}
}

async resolve (id: string, parent: string, job: Job, cjsResolve: boolean): Promise<string | string[]> {
return resolveDependency(id, parent, job, cjsResolve);
}
Expand Down Expand Up @@ -317,8 +356,11 @@ export class Job {
if (path.endsWith('.json')) return;
if (path.endsWith('.node')) return await sharedLibEmit(path, this);

// js files require the "type": "module" lookup, so always emit the package.json
if (path.endsWith('.js')) {
// .js and .ts files can change behavior based on { "type": "module" }
// in the nearest package.json so we must emit it too. We don't need to
// emit for .cjs/.mjs/.cts/.mts files since their behavior does not
// depend on package.json
if (path.endsWith('.js') || path.endsWith('.ts')) {
const pjsonBoundary = await this.getPjsonBoundary(path);
if (pjsonBoundary)
await this.emitFile(pjsonBoundary + sep + 'package.json', 'resolve', path);
Expand All @@ -342,8 +384,9 @@ export class Job {

const { deps, imports, assets, isESM } = analyzeResult;

if (isESM)
if (isESM) {
this.esmFileList.add(relative(this.base, path));
}

await Promise.all([
...[...assets].map(async asset => {
Expand All @@ -354,48 +397,8 @@ export class Job {
else
await this.emitFile(asset, 'asset', path);
}),
...[...deps].map(async dep => {
try {
var resolved = await this.resolve(dep, path, this, !isESM);
}
catch (e: any) {
this.warnings.add(new Error(`Failed to resolve dependency ${dep}:\n${e && e.message}`));
return;
}
if (Array.isArray(resolved)) {
for (const item of resolved) {
// ignore builtins
if (item.startsWith('node:')) return;
await this.emitDependency(item, path);
}
}
else {
// ignore builtins
if (resolved.startsWith('node:')) return;
await this.emitDependency(resolved, path);
}
}),
...[...imports].map(async dep => {
try {
var resolved = await this.resolve(dep, path, this, false);
}
catch (e: any) {
this.warnings.add(new Error(`Failed to resolve dependency ${dep}:\n${e && e.message}`));
return;
}
if (Array.isArray(resolved)) {
for (const item of resolved) {
// ignore builtins
if (item.startsWith('node:')) return;
await this.emitDependency(item, path);
}
}
else {
// ignore builtins
if (resolved.startsWith('node:')) return;
await this.emitDependency(resolved, path);
}
})
...[...deps].map(async dep => this.maybeEmitDep(dep, path, !isESM)),
...[...imports].map(async dep => this.maybeEmitDep(dep, path, false)),
]);
}
}
2 changes: 1 addition & 1 deletion src/resolve-dependency.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ async function resolveDir (path: string, parent: string, job: Job) {
return resolveFile(resolve(path, 'index'), parent, job);
}

class NotFoundError extends Error {
export class NotFoundError extends Error {
public code: string;
constructor(specifier: string, parent: string) {
super("Cannot find module '" + specifier + "' loaded from " + parent);
Expand Down
3 changes: 3 additions & 0 deletions test/unit.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,9 @@ for (const { testName, isRoot } of unitTests) {
if (testName === "tsx-input") {
inputFileNames = ["input.tsx"];
}
if (testName === "ts-input-esm") {
inputFileNames = ["input.ts"];
}
if (testName === "processed-dependency" && cached) {
inputFileNames = ["input-cached.js"]
outputFileName = "output-cached.js"
Expand Down
8 changes: 8 additions & 0 deletions test/unit/ts-input-esm/dep1.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
async function start() {
const { dep2 } = await import('./dep2.js');
return dep2;
}

start();

export const dep1 = 'dep1';
1 change: 1 addition & 0 deletions test/unit/ts-input-esm/dep2.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const dep2 = 'dep2';
1 change: 1 addition & 0 deletions test/unit/ts-input-esm/input.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import { dep1 } from './dep1.js';
6 changes: 6 additions & 0 deletions test/unit/ts-input-esm/output.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
[
"test/unit/ts-input-esm/dep1.ts",
"test/unit/ts-input-esm/dep2.ts",
"test/unit/ts-input-esm/input.ts",
"test/unit/ts-input-esm/package.json"
]
4 changes: 4 additions & 0 deletions test/unit/ts-input-esm/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"private": true,
"type": "module"
}
10 changes: 10 additions & 0 deletions test/unit/ts-input-esm/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"compilerOptions": {
"target": "esnext",
"module": "esnext",
"lib": ["esnext"],
"moduleResolution": "node",
"esModuleInterop": true,
"strict": true
}
}

0 comments on commit 40a8710

Please sign in to comment.