analyzeDependencies: Provide a hint when we can't find an export that exists as a local binding
MODIFY
packages/@romejs/core/common/types/analyzeDependencies.ts
packages/@romejs/core/common/types/analyzeDependencies.ts
@@ -12,6 +12,7 @@ import {
ConstProgramSyntax,
} from '@romejs/js-ast';
import {SourceLocation} from '@romejs/parser-core';
+import {Dict} from '@romejs/typescript-helpers';
export type AnalyzeModuleType = 'es' | 'cjs' | 'unknown';
@@ -71,7 +72,12 @@ export type AnalyzeDependencyImportFirstUsage = Array<
AnalyzeDependencyImportUsageItem
>;
+export type AnalyzeDependencyTopLevelLocalBindings = Dict<
+ | undefined
+ | SourceLocation>;
+
export type AnalyzeDependencyResult = {
+ topLevelLocalBindings: AnalyzeDependencyTopLevelLocalBindings;
moduleType: AnalyzeModuleType;
syntax: Array<ConstProgramSyntax>;
diagnostics: Diagnostics;
@@ -82,6 +88,7 @@ export type AnalyzeDependencyResult = {
};
export const UNKNOWN_ANALYZE_DEPENDENCIES_RESULT: AnalyzeDependencyResult = {
+ topLevelLocalBindings: {},
moduleType: 'unknown',
syntax: [],
diagnostics: [],

MODIFY
packages/@romejs/core/master/bundler/BundleRequest.ts
packages/@romejs/core/master/bundler/BundleRequest.ts
@@ -101,6 +101,7 @@ export default class BundleRequest {
paths: [this.resolvedEntry],
diagnosticsProcessor: this.diagnostics,
analyzeProgress,
+ validate: true,
});
} finally {
analyzeProgress.end();
@@ -142,7 +143,7 @@ export default class BundleRequest {
// Build a map of relative module sources to module id
const relativeSourcesToModuleId: Dict<string> = {};
for (const [relative, absolute] of mod.relativeToAbsolutePath) {
- const moduleId = graph.getNode(absolute).id;
+ const moduleId = graph.getNode(absolute).uid;
relativeSourcesToModuleId[relative] = moduleId;
}
@@ -168,7 +169,7 @@ export default class BundleRequest {
const opts: WorkerBundleCompileOptions = {
mode: this.mode,
moduleAll: mod.all,
- moduleId: mod.id,
+ moduleId: mod.uid,
relativeSourcesToModuleId,
resolvedImports,
assetPath,
@@ -284,7 +285,7 @@ export default class BundleRequest {
declaredCJS.add(module);
- push(` var ${getPrefixedBundleNamespace(module.id)} = {};`);
+ push(` var ${getPrefixedBundleNamespace(module.uid)} = {};`);
}
// Add on files
@@ -303,12 +304,12 @@ export default class BundleRequest {
// Only do this in modern mode, the module id will already be in the wrapper otherwise
if (mode === 'modern') {
- push(` // ${module.id}`);
+ push(` // ${module.uid}`);
}
declareCJS(module);
- addMappings(module.id, compileResult.sourceText, compileResult.mappings);
+ addMappings(module.uid, compileResult.sourceText, compileResult.mappings);
push(compileResult.compiledCode);
push('');
}
@@ -316,9 +317,9 @@ export default class BundleRequest {
// push on initial entry require
const entryModule = graph.getNode(resolvedEntry);
if (mode === 'modern') {
- push(` return ${getPrefixedBundleNamespace(entryModule.id)};`);
+ push(` return ${getPrefixedBundleNamespace(entryModule.uid)};`);
} else {
- push(` return Rome.requireNamespace("${entryModule.id}");`);
+ push(` return Rome.requireNamespace("${entryModule.uid}");`);
}
// push footer

MODIFY
packages/@romejs/core/master/dependencies/DependencyGraph.ts
packages/@romejs/core/master/dependencies/DependencyGraph.ts
@@ -134,10 +134,10 @@ export default class DependencyGraph {
const stats: BundleBuddyStats = [];
for (const node of this.nodes.values()) {
- const source = node.id;
+ const source = node.uid;
for (const absoluteTarget of node.relativeToAbsolutePath.values()) {
- const target = this.getNode(absoluteTarget).id;
+ const target = this.getNode(absoluteTarget).uid;
stats.push({
target,
source,
@@ -146,7 +146,7 @@ export default class DependencyGraph {
}
for (const absoluteEntry of entries) {
- const source = this.getNode(absoluteEntry).id;
+ const source = this.getNode(absoluteEntry).uid;
stats.push({
source,
target: undefined,
@@ -157,9 +157,11 @@ export default class DependencyGraph {
}
addNode(filename: AbsoluteFilePath, res: WorkerAnalyzeDependencyResult) {
- const module = new DependencyNode(this, this.master.projectManager.getUid(
- filename,
- ), filename, res);
+ const module = new DependencyNode(
+ this,
+ this.master.projectManager.getFileReference(filename),
+ res,
+ );
this.nodes.set(filename, module);
return module;
}

MODIFY
packages/@romejs/core/master/dependencies/DependencyNode.ts
packages/@romejs/core/master/dependencies/DependencyNode.ts
@@ -9,7 +9,12 @@ import DependencyGraph from './DependencyGraph';
import {BundleCompileResolvedImports} from '@romejs/js-compiler';
import {ConstImportModuleKind} from '@romejs/js-ast';
import {SourceLocation} from '@romejs/parser-core';
-import {Diagnostics, Diagnostic, descriptions} from '@romejs/diagnostics';
+import {
+ Diagnostics,
+ Diagnostic,
+ descriptions,
+ DiagnosticLocation,
+} from '@romejs/diagnostics';
import {ProjectDefinition} from '@romejs/project';
import {DependencyOrder} from './DependencyOrderer';
import DependencyOrderer from './DependencyOrderer';
@@ -22,6 +27,7 @@ import {
AnalyzeDependencyName,
AnalyzeExportLocal,
} from '@romejs/core';
+import {FileReference} from '@romejs/core/common/types/files';
type ResolvedImportFound = {
type: 'FOUND';
@@ -69,15 +75,15 @@ type DependencyNodeDependency = {
export default class DependencyNode {
constructor(
graph: DependencyGraph,
- id: string,
- path: AbsoluteFilePath,
+ ref: FileReference,
res: WorkerAnalyzeDependencyResult,
) {
this.graph = graph;
- this.project = graph.master.projectManager.assertProjectExisting(path);
- this.path = path;
- this.id = id;
+ this.project = graph.master.projectManager.assertProjectExisting(ref.real);
+ this.uid = ref.uid;
+ this.path = ref.real;
+ this.ref = ref;
this.type = res.moduleType;
this.usedAsync = false;
@@ -87,7 +93,7 @@ export default class DependencyNode {
this.analyze = res;
- const {handler} = getFileHandler(path, this.project.config);
+ const {handler} = getFileHandler(ref.real, this.project.config);
this.handler = handler;
}
@@ -98,7 +104,8 @@ export default class DependencyNode {
type: AnalyzeModuleType;
project: ProjectDefinition;
path: AbsoluteFilePath;
- id: string;
+ uid: string;
+ ref: FileReference;
all: boolean;
usedAsync: boolean;
handler: undefined | ExtensionHandler;
@@ -196,6 +203,24 @@ export default class DependencyNode {
return orderer.order(this.path);
}
+ // Get a list of all DependencyNodes where exports could be resolved. eg. `export *`
+ getExportedModules(chain: Set<DependencyNode> = new Set()): Set<DependencyNode> {
+ if (chain.has(this)) {
+ return new Set();
+ } else {
+ chain.add(this);
+ }
+
+ for (const exp of this.analyze.exports) {
+ if (exp.type === 'externalAll') {
+ const node = this.getNodeFromRelativeDependency(exp.source);
+ node.getExportedModules(chain);
+ }
+ }
+
+ return chain;
+ }
+
getExportedNames(
kind: ConstImportModuleKind,
seen: Set<DependencyNode> = new Set(),
@@ -242,10 +267,39 @@ export default class DependencyNode {
kind: ConstImportModuleKind,
resolved: ResolvedImportNotFound,
): Diagnostic {
+ const location: DiagnosticLocation = {
+ ...resolved.loc,
+ mtime: this.getMtime(),
+ };
+
+ const expectedName = resolved.name;
+ const fromSource = resolved.node.uid;
+
+ // Check if there was a matching local in any of the exported modules
+ for (const mod of resolved.node.getExportedModules()) {
+ // We use an object as a hash map so need to check for pollution
+ if (Object.prototype.hasOwnProperty.call(
+ mod.analyze.topLevelLocalBindings,
+ expectedName,
+ )) {
+ const localLoc = mod.analyze.topLevelLocalBindings[expectedName];
+ if (localLoc !== undefined) {
+ return {
+ description: descriptions.BUNDLER.UNKNOWN_EXPORT_POSSIBLE_UNEXPORTED_LOCAL(
+ expectedName,
+ fromSource,
+ localLoc,
+ ),
+ location,
+ };
+ }
+ }
+ }
+
return {
description: descriptions.BUNDLER.UNKNOWN_EXPORT(
- resolved.name,
- resolved.node.id,
+ expectedName,
+ fromSource,
Array.from(resolved.node.getExportedNames(kind)),
(name: string) => {
const exportInfo = resolved.node.resolveImport(name, undefined);
@@ -258,15 +312,12 @@ export default class DependencyNode {
return {
location: exportInfo.record.loc,
- source: exportInfo.node !== resolved.node
- ? exportInfo.node.path.join() : undefined,
+ source: exportInfo.node === resolved.node
+ ? undefined : exportInfo.node.path.join(),
};
},
),
- location: {
- ...resolved.loc,
- mtime: this.getMtime(),
- },
+ location,
};
}
@@ -281,7 +332,7 @@ export default class DependencyNode {
return {
description: descriptions.BUNDLER.IMPORT_TYPE_MISMATCH(
name,
- node.id,
+ node.uid,
kind,
record.kind,
record.loc,
@@ -347,9 +398,9 @@ export default class DependencyNode {
}
// If the resolved target isn't the same as the file then forward it
- if (resolved.node.id !== mod.id) {
- resolvedImports[`${mod.id}:${name}`] = {
- id: resolved.node.id,
+ if (resolved.node.uid !== mod.uid) {
+ resolvedImports[`${mod.uid}:${name}`] = {
+ id: resolved.node.uid,
name: resolved.record.name,
};
}

MODIFY
packages/@romejs/diagnostics/descriptions.ts
packages/@romejs/diagnostics/descriptions.ts
@@ -739,6 +739,27 @@ export const descriptions = createMessages({
},
}),
}),
+
+ UNKNOWN_EXPORT_POSSIBLE_UNEXPORTED_LOCAL: (
+ name: string,
+ source: string,
+ location: DiagnosticLocation,
+ ) =>
+ ({
+ message: markup`Couldn't find export <emphasis>${name}</emphasis> in <filelink emphasis target="${source}" />`,
+ category: 'bundler/unknownExport',
+ advice: [
+ {
+ type: 'log',
+ category: 'info',
+ message: 'However we found a matching local variable in this module. Did you forget to export it?',
+ },
+ {
+ type: 'frame',
+ location,
+ },
+ ],
+ }),
},
SPDX: {

MODIFY
packages/@romejs/js-compiler/api/analyzeDependencies.test.md
packages/@romejs/js-compiler/api/analyzeDependencies.test.md
@@ -11,6 +11,7 @@ Object {
importFirstUsage: Array []
moduleType: 'es'
syntax: Array []
+ topLevelLocalBindings: Object {}
exports: Array [
local {
kind: 'value'
@@ -69,6 +70,21 @@ Object {
importFirstUsage: Array []
moduleType: 'es'
syntax: Array []
+ topLevelLocalBindings: Object {
+ foo: Object {
+ filename: 'unknown'
+ end: Object {
+ column: 22
+ index: 23
+ line: 2
+ }
+ start: Object {
+ column: 19
+ index: 20
+ line: 2
+ }
+ }
+ }
exports: Array [
local {
kind: 'value'
@@ -102,6 +118,7 @@ Object {
importFirstUsage: Array []
moduleType: 'es'
syntax: Array []
+ topLevelLocalBindings: Object {}
dependencies: Array [
es {
kind: 'value'
@@ -139,6 +156,88 @@ Object {
importFirstUsage: Array []
moduleType: 'es'
syntax: Array []
+ topLevelLocalBindings: Object {}
+}
+```
+
+## `defines topLevelLocalBindings`
+
+```javascript
+Object {
+ diagnostics: Array []
+ exports: Array []
+ firstTopAwaitLocation: undefined
+ importFirstUsage: Array []
+ moduleType: 'es'
+ syntax: Array []
+ topLevelLocalBindings: Object {
+ bar: Object {
+ filename: 'unknown'
+ end: Object {
+ column: 15
+ index: 16
+ line: 2
+ }
+ start: Object {
+ column: 12
+ index: 13
+ line: 2
+ }
+ }
+ foo: Object {
+ filename: 'unknown'
+ end: Object {
+ column: 13
+ index: 43
+ line: 3
+ }
+ start: Object {
+ column: 10
+ index: 40
+ line: 3
+ }
+ }
+ }
+ dependencies: Array [
+ es {
+ kind: 'value'
+ all: false
+ async: false
+ optional: false
+ source: 'foo'
+ loc: Object {
+ filename: 'unknown'
+ end: Object {
+ column: 27
+ index: 28
+ line: 2
+ }
+ start: Object {
+ column: 22
+ index: 23
+ line: 2
+ }
+ }
+ names: Array [
+ value {
+ name: 'bar'
+ loc: Object {
+ filename: 'unknown'
+ end: Object {
+ column: 15
+ index: 16
+ line: 2
+ }
+ start: Object {
+ column: 12
+ index: 13
+ line: 2
+ }
+ }
+ }
+ ]
+ }
+ ]
}
```
@@ -151,6 +250,21 @@ Object {
importFirstUsage: Array []
moduleType: 'cjs'
syntax: Array []
+ topLevelLocalBindings: Object {
+ foo: Object {
+ filename: 'unknown'
+ end: Object {
+ column: 22
+ index: 23
+ line: 2
+ }
+ start: Object {
+ column: 19
+ index: 20
+ line: 2
+ }
+ }
+ }
diagnostics: Array [
Object {
origins: Array [Object {category: 'js-parser'}]
@@ -241,6 +355,21 @@ Object {
importFirstUsage: Array []
moduleType: 'es'
syntax: Array []
+ topLevelLocalBindings: Object {
+ yes: Object {
+ filename: 'unknown'
+ end: Object {
+ column: 18
+ index: 43
+ line: 4
+ }
+ start: Object {
+ column: 15
+ index: 40
+ line: 4
+ }
+ }
+ }
dependencies: Array [
es {
kind: 'value'
@@ -298,6 +427,7 @@ Object {
importFirstUsage: Array []
moduleType: 'cjs'
syntax: Array []
+ topLevelLocalBindings: Object {}
exports: Array [
local {
kind: 'value'
@@ -337,6 +467,7 @@ Object {
importFirstUsage: Array []
moduleType: 'cjs'
syntax: Array []
+ topLevelLocalBindings: Object {}
exports: Array [
local {
kind: 'value'
@@ -376,6 +507,47 @@ Object {
importFirstUsage: Array []
moduleType: 'es'
syntax: Array []
+ topLevelLocalBindings: Object {
+ Bar: Object {
+ filename: 'unknown'
+ end: Object {
+ column: 22
+ index: 83
+ line: 4
+ }
+ start: Object {
+ column: 19
+ index: 80
+ line: 4
+ }
+ }
+ foo: Object {
+ filename: 'unknown'
+ end: Object {
+ column: 25
+ index: 55
+ line: 3
+ }
+ start: Object {
+ column: 22
+ index: 52
+ line: 3
+ }
+ }
+ yes: Object {
+ filename: 'unknown'
+ end: Object {
+ column: 22
+ index: 23
+ line: 2
+ }
+ start: Object {
+ column: 19
+ index: 20
+ line: 2
+ }
+ }
+ }
exports: Array [
local {
kind: 'value'
@@ -445,6 +617,7 @@ Object {
importFirstUsage: Array []
moduleType: 'es'
syntax: Array []
+ topLevelLocalBindings: Object {}
exports: Array [
local {
kind: 'value'
@@ -477,6 +650,7 @@ Object {
importFirstUsage: Array []
moduleType: 'es'
syntax: Array []
+ topLevelLocalBindings: Object {}
exports: Array [
external {
kind: 'value'
@@ -655,6 +829,7 @@ Object {
importFirstUsage: Array []
moduleType: 'es'
syntax: Array []
+ topLevelLocalBindings: Object {}
exports: Array [
externalAll {
kind: 'value'
@@ -710,6 +885,21 @@ Object {
importFirstUsage: Array []
moduleType: 'es'
syntax: Array []
+ topLevelLocalBindings: Object {
+ bar: Object {
+ filename: 'unknown'
+ end: Object {
+ column: 16
+ index: 17
+ line: 2
+ }
+ start: Object {
+ column: 13
+ index: 14
+ line: 2
+ }
+ }
+ }
dependencies: Array [
es {
kind: 'value'
@@ -763,6 +953,60 @@ Object {
importFirstUsage: Array []
moduleType: 'es'
syntax: Array []
+ topLevelLocalBindings: Object {
+ bar: Object {
+ filename: 'unknown'
+ end: Object {
+ column: 17
+ index: 18
+ line: 2
+ }
+ start: Object {
+ column: 14
+ index: 15
+ line: 2
+ }
+ }
+ foo: Object {
+ filename: 'unknown'
+ end: Object {
+ column: 22
+ index: 23
+ line: 2
+ }
+ start: Object {
+ column: 19
+ index: 20
+ line: 2
+ }
+ }
+ lol: Object {
+ filename: 'unknown'
+ end: Object {
+ column: 38
+ index: 39
+ line: 2
+ }
+ start: Object {
+ column: 35
+ index: 36
+ line: 2
+ }
+ }
+ to: Object {
+ filename: 'unknown'
+ end: Object {
+ column: 48
+ index: 49
+ line: 2
+ }
+ start: Object {
+ column: 46
+ index: 47
+ line: 2
+ }
+ }
+ }
dependencies: Array [
es {
kind: 'value'
@@ -864,6 +1108,7 @@ Object {
importFirstUsage: Array []
moduleType: 'es'
syntax: Array []
+ topLevelLocalBindings: Object {}
dependencies: Array [
es {
kind: 'value'
@@ -900,6 +1145,7 @@ Object {
importFirstUsage: Array []
moduleType: 'es'
syntax: Array []
+ topLevelLocalBindings: Object {}
exports: Array [
local {
kind: 'value'
@@ -969,6 +1215,7 @@ Object {
importFirstUsage: Array []
moduleType: 'es'
syntax: Array []
+ topLevelLocalBindings: Object {}
dependencies: Array [
es {
kind: 'value'
@@ -1026,6 +1273,7 @@ Object {
importFirstUsage: Array []
moduleType: 'es'
syntax: Array []
+ topLevelLocalBindings: Object {}
firstTopAwaitLocation: Object {
filename: 'unknown'
end: Object {
@@ -1060,6 +1308,21 @@ Object {
valueType: 'other'
}
]
+ topLevelLocalBindings: Object {
+ yes: Object {
+ filename: 'unknown'
+ end: Object {
+ column: 18
+ index: 90
+ line: 7
+ }
+ start: Object {
+ column: 15
+ index: 87
+ line: 7
+ }
+ }
+ }
}
```
@@ -1074,5 +1337,6 @@ Object {
importFirstUsage: Array []
moduleType: 'es'
syntax: Array []
+ topLevelLocalBindings: Object {}
}
```

MODIFY
packages/@romejs/js-compiler/api/analyzeDependencies.test.ts
packages/@romejs/js-compiler/api/analyzeDependencies.test.ts
@@ -165,3 +165,10 @@ test('disallow mix of es and cjs exports', async (t) => {
exports.bar = 'foo';
`, 'script'));
});
+
+test('defines topLevelLocalBindings', async (t) => {
+ t.snapshot(await testAnalyzeDeps(`
+ import {bar} from 'foo';
+ const foo = 'bar';
+ `, 'module'));
+});

MODIFY
packages/@romejs/js-compiler/api/analyzeDependencies/index.ts
packages/@romejs/js-compiler/api/analyzeDependencies/index.ts
@@ -28,6 +28,7 @@ import {
AnalyzeDependencyName,
AnalyzeDependencyImportFirstUsage,
AnalyzeModuleType,
+ AnalyzeDependencyTopLevelLocalBindings,
} from '@romejs/core';
import {descriptions} from '@romejs/diagnostics';
@@ -250,7 +251,15 @@ export default async function analyzeDependencies(
});
}
+ const topLevelLocalBindings: AnalyzeDependencyTopLevelLocalBindings = {};
+
+ // Get all top level bindings
+ for (const [name, binding] of context.getRootScope().evaluate(ast).getOwnBindings()) {
+ topLevelLocalBindings[name] = binding.node.loc;
+ }
+
const res: AnalyzeDependencyResult = {
+ topLevelLocalBindings,
moduleType,
firstTopAwaitLocation,
exports,

analyzeDependencies: Provide a hint when we can't find an export that exists as a local binding