Skip to content

Commit

Permalink
fix: resolver cases (#386)
Browse files Browse the repository at this point in the history
  • Loading branch information
guybedford authored Oct 6, 2024
1 parent ac746e4 commit 3a7d2c9
Show file tree
Hide file tree
Showing 9 changed files with 54 additions and 33 deletions.
1 change: 1 addition & 0 deletions src/install/installer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,7 @@ export class Installer {
) {
this.log = log;
this.resolver = resolver;
this.resolver.installer = this;
this.resolutions = opts.resolutions || {};
this.installBaseUrl = baseUrl;
this.opts = opts;
Expand Down
52 changes: 26 additions & 26 deletions src/trace/resolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ export class Resolver {
traceTs: boolean;
traceSystem: boolean;
context: Record<string, any>;
installer: Installer;
constructor({
env,
log,
Expand Down Expand Up @@ -471,10 +472,7 @@ export class Resolver {
const subpath = "./" + url.slice(pkgUrl.length);
if (subpath in pcfg.browser) {
const target = pcfg.browser[subpath];
if (target === false)
throw new Error(
`TODO: Empty browser map for ${subpath} in ${url}`
);
if (target === false) return this.resolveEmpty(parentIsCjs, url);
if (!target.startsWith("./"))
throw new Error(
`TODO: External browser map for ${subpath} to ${target} in ${url}`
Expand Down Expand Up @@ -720,14 +718,36 @@ export class Resolver {
return null;
}

async resolveEmpty(cjsEnv: boolean, originalSpecifier: string, parentUrl?: URL) {
const stdlibTarget = {
registry: "npm",
name: "@jspm/core",
ranges: [new SemverRange("*")],
unstable: true,
};
const provider = this.installer.getProvider(stdlibTarget);
const pkg = await this.resolveLatestTarget(
stdlibTarget,
provider,
parentUrl?.href
);
return this.resolveExport(
await this.pkgToUrl(pkg, provider),
"./nodelibs/@empty",
cjsEnv,
false,
originalSpecifier,
parentUrl
);
}

// Note: updates here must be tracked in function above
async resolveExport(
pkgUrl: `${string}/`,
subpath: `.${string}`,
cjsEnv: boolean,
parentIsCjs: boolean,
originalSpecifier: string,
installer: Installer,
parentUrl?: URL
): Promise<string> {
const env = cjsEnv ? this.cjsEnv : this.env;
Expand All @@ -739,27 +759,7 @@ export class Resolver {
pcfg.exports !== null &&
Object.keys(pcfg.exports).length === 0
) {
const stdlibTarget = {
registry: "npm",
name: "@jspm/core",
ranges: [new SemverRange("*")],
unstable: true,
};
const provider = installer.getProvider(stdlibTarget);
const pkg = await this.resolveLatestTarget(
stdlibTarget,
provider,
parentUrl.href
);
return this.resolveExport(
await this.pkgToUrl(pkg, provider),
"./nodelibs/@empty",
cjsEnv,
parentIsCjs,
originalSpecifier,
installer,
parentUrl
);
return this.resolveEmpty(cjsEnv, originalSpecifier, parentUrl);
}

function throwExportNotDefined() {
Expand Down
6 changes: 3 additions & 3 deletions src/trace/tracemap.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,8 @@ function combineSubpaths(
installSubpath: "." | `./${string}` | null,
traceSubpath: "." | `./${string}`
): `./${string}` | "." {
if (traceSubpath.endsWith('/'))
throw new Error('Trailing slash subpaths unsupported');
return installSubpath === null ||
installSubpath === "." ||
traceSubpath === "."
Expand Down Expand Up @@ -508,7 +510,6 @@ export default class TraceMap {
cjsEnv,
parentIsCjs,
specifier,
this.installer,
new URL(parentUrl)
)
);
Expand Down Expand Up @@ -561,11 +562,10 @@ export default class TraceMap {
const resolved = await this.resolver.realPath(
await this.resolver.resolveExport(
installUrl,
combineSubpaths(installSubpath, subpath),
combineSubpaths(installSubpath, parentIsCjs && subpath.endsWith('/') ? subpath.slice(0, -1) as `./${string}` : subpath),
cjsEnv,
parentIsCjs,
specifier,
this.installer,
new URL(parentUrl)
)
);
Expand Down
1 change: 1 addition & 0 deletions test/resolve/cjspkg/browser-dep-exclude.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import 'excluded';
1 change: 1 addition & 0 deletions test/resolve/cjspkg/browser-dep-include.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
import 'jquery';
2 changes: 2 additions & 0 deletions test/resolve/cjspkg/browser.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import './browser-dep-exclude.js';
import './browser-dep-include.js';
3 changes: 3 additions & 0 deletions test/resolve/cjspkg/package.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
{
"dependencies": {
"process": "^0.11.10"
},
"browser": {
"./browser-dep-exclude.js": false
}
}
13 changes: 13 additions & 0 deletions test/resolve/unused-cjs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import assert from "assert";

const generator = new Generator({
mapUrl: import.meta.url,
commonJS: true
});

// Should not throw, index file doesn't use CJS:
Expand All @@ -15,3 +16,15 @@ await (async () => {
assert(false);
} catch {}
})();

await generator.install({ target: './cjspkg', subpath: './browser.js' });
assert.deepStrictEqual(generator.getMap(), {
imports: {
'./cjspkg/browser-dep-exclude.js': 'https://ga.jspm.io/npm:@jspm/core@2.0.1/nodelibs/@empty.js',
'cjspkg/browser.js': './cjspkg/browser.js',
unusedcjspkg: './unusedcjspkg/index.js'
},
scopes: {
'./cjspkg/': { jquery: 'https://ga.jspm.io/npm:jquery@3.7.1/dist/jquery.js' }
}
});
8 changes: 4 additions & 4 deletions test/test.html
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"#lib/config/files/index.js": "https://ga.jspm.io/npm:@babel/core@7.25.2/lib/config/files/index-browser.js",
"#lib/config/resolve-targets.js": "https://ga.jspm.io/npm:@babel/core@7.25.2/lib/config/resolve-targets-browser.js",
"#lib/transform-file.js": "https://ga.jspm.io/npm:@babel/core@7.25.2/lib/transform-file-browser.js",
"#node.js": "https://ga.jspm.io/npm:browserslist@4.23.3/browser.js",
"#node.js": "https://ga.jspm.io/npm:browserslist@4.24.0/browser.js",
"@ampproject/remapping": "https://ga.jspm.io/npm:@ampproject/remapping@2.3.0/dist/remapping.umd.js",
"@babel/code-frame": "https://ga.jspm.io/npm:@babel/code-frame@7.24.7/lib/index.js",
"@babel/compat-data/native-modules": "https://ga.jspm.io/npm:@babel/compat-data@7.25.4/native-modules.js",
Expand Down Expand Up @@ -63,15 +63,15 @@
"@jridgewell/sourcemap-codec": "https://ga.jspm.io/npm:@jridgewell/sourcemap-codec@1.5.0/dist/sourcemap-codec.umd.js",
"@jridgewell/trace-mapping": "https://ga.jspm.io/npm:@jridgewell/trace-mapping@0.3.25/dist/trace-mapping.umd.js",
"ansi-styles": "https://ga.jspm.io/npm:ansi-styles@3.2.1/index.js",
"browserslist": "https://ga.jspm.io/npm:browserslist@4.23.3/index.js",
"browserslist": "https://ga.jspm.io/npm:browserslist@4.24.0/index.js",
"buffer": "https://ga.jspm.io/npm:@jspm/core@2.0.1/nodelibs/browser/buffer.js",
"caniuse-lite/dist/unpacker/agents": "https://ga.jspm.io/npm:caniuse-lite@1.0.30001658/dist/unpacker/agents.js",
"caniuse-lite/dist/unpacker/agents": "https://ga.jspm.io/npm:caniuse-lite@1.0.30001664/dist/unpacker/agents.js",
"chalk": "https://ga.jspm.io/npm:chalk@2.4.2/index.js",
"color-convert": "https://ga.jspm.io/npm:color-convert@1.9.3/index.js",
"color-name": "https://ga.jspm.io/npm:color-name@1.1.3/index.js",
"convert-source-map": "https://ga.jspm.io/npm:convert-source-map@2.0.0/index.js",
"debug": "https://ga.jspm.io/npm:debug@4.3.7/src/browser.js",
"electron-to-chromium/versions": "https://ga.jspm.io/npm:electron-to-chromium@1.5.18/versions.js",
"electron-to-chromium/versions": "https://ga.jspm.io/npm:electron-to-chromium@1.5.29/versions.js",
"escape-string-regexp": "https://ga.jspm.io/npm:escape-string-regexp@1.0.5/index.js",
"fs": "https://ga.jspm.io/npm:@jspm/core@2.0.1/nodelibs/browser/fs.js",
"gensync": "https://ga.jspm.io/npm:gensync@1.0.0-beta.2/index.js",
Expand Down

0 comments on commit 3a7d2c9

Please sign in to comment.