Skip to content

Commit

Permalink
Warn for inconsistent import assertions
Browse files Browse the repository at this point in the history
  • Loading branch information
lukastaegert committed Oct 9, 2022
1 parent 518a6d8 commit dc486f3
Show file tree
Hide file tree
Showing 8 changed files with 184 additions and 36 deletions.
38 changes: 27 additions & 11 deletions src/Module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import ExportAllDeclaration from './ast/nodes/ExportAllDeclaration';
import ExportDefaultDeclaration from './ast/nodes/ExportDefaultDeclaration';
import type ExportNamedDeclaration from './ast/nodes/ExportNamedDeclaration';
import type Identifier from './ast/nodes/Identifier';
import ImportAttribute from './ast/nodes/ImportAttribute';
import type ImportDeclaration from './ast/nodes/ImportDeclaration';
import type ImportExpression from './ast/nodes/ImportExpression';
import Literal from './ast/nodes/Literal';
Expand Down Expand Up @@ -52,6 +51,7 @@ import {
augmentCodeLocation,
errAmbiguousExternalNamespaces,
errCircularReexport,
errInconsistentImportAssertions,
errInvalidFormatForTopLevelAwait,
errInvalidSourcemapForError,
errMissingExport,
Expand All @@ -66,7 +66,10 @@ import { getId } from './utils/getId';
import { getOrCreate } from './utils/getOrCreate';
import { getOriginalLocation } from './utils/getOriginalLocation';
import { makeLegal } from './utils/identifierHelpers';
import { getAssertionsFromImportExportDeclaration } from './utils/parseAssertions';
import {
doAssertionsDiffer,
getAssertionsFromImportExportDeclaration
} from './utils/parseAssertions';
import { basename, extname } from './utils/path';
import type { RenderOptions } from './utils/renderHelpers';
import { timeEnd, timeStart } from './utils/timers';
Expand Down Expand Up @@ -257,7 +260,8 @@ export default class Module {
isEntry: boolean,
moduleSideEffects: boolean | 'no-treeshake',
syntheticNamedExports: boolean | string,
meta: CustomPluginOptions
meta: CustomPluginOptions,
assertions: Record<string, string>
) {
this.excludeFromSourcemap = /\0/.test(id);
this.context = options.moduleContext(id);
Expand All @@ -276,8 +280,7 @@ export default class Module {
} = this;

this.info = {
// TODO Lukas use correct assertions
assertions: EMPTY_OBJECT,
assertions,
ast: null,
code: null,
get dynamicallyImportedIdResolutions() {
Expand Down Expand Up @@ -911,7 +914,7 @@ export default class Module {
});
} else if (node instanceof ExportAllDeclaration) {
const source = node.source.value;
this.addSource(source, node.assertions);
this.addSource(source, node);
if (node.exported) {
// export * as name from './other'

Expand All @@ -931,7 +934,7 @@ export default class Module {
// export { name } from './other'

const source = node.source.value;
this.addSource(source, node.assertions);
this.addSource(source, node);
for (const specifier of node.specifiers) {
const name = specifier.exported.name;
this.reexportDescriptions.set(name, {
Expand Down Expand Up @@ -971,7 +974,7 @@ export default class Module {

private addImport(node: ImportDeclaration): void {
const source = node.source.value;
this.addSource(source, node.assertions);
this.addSource(source, node);
for (const specifier of node.specifiers) {
const isDefault = specifier.type === NodeType.ImportDefaultSpecifier;
const isNamespace = specifier.type === NodeType.ImportNamespaceSpecifier;
Expand Down Expand Up @@ -1050,9 +1053,22 @@ export default class Module {
addSideEffectDependencies(alwaysCheckedDependencies);
}

private addSource(source: string, assertions: ImportAttribute[] | undefined) {
// TODO Lukas handle existing and conflicting sources
this.sourcesWithAssertions.set(source, getAssertionsFromImportExportDeclaration(assertions));
private addSource(
source: string,
declaration: ImportDeclaration | ExportNamedDeclaration | ExportAllDeclaration
) {
const parsedAssertions = getAssertionsFromImportExportDeclaration(declaration.assertions);
const existingAssertions = this.sourcesWithAssertions.get(source);
if (existingAssertions) {
if (doAssertionsDiffer(existingAssertions, parsedAssertions)) {
this.warn(
errInconsistentImportAssertions(existingAssertions, parsedAssertions, source, this.id),
declaration.start
);
}
} else {
this.sourcesWithAssertions.set(source, parsedAssertions);
}
}

private getVariableFromNamespaceReexports(
Expand Down
74 changes: 49 additions & 25 deletions src/ModuleLoader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import {
errEntryCannotBeExternal,
errExternalSyntheticExports,
errImplicitDependantCannotBeExternal,
errInconsistentImportAssertions,
errInternalIdCannotBeExternal,
error,
errUnresolvedEntry,
Expand All @@ -30,7 +31,7 @@ import {
errUnresolvedImportTreatedAsExternal
} from './utils/error';
import { promises as fs } from './utils/fs';
import { getAssertionsFromImportExpression } from './utils/parseAssertions';
import { doAssertionsDiffer, getAssertionsFromImportExpression } from './utils/parseAssertions';
import { isAbsolute, isRelative, resolve } from './utils/path';
import relativeId from './utils/relativeId';
import { resolveId } from './utils/resolveId';
Expand Down Expand Up @@ -184,7 +185,6 @@ export class ModuleLoader {
resolvedId: { id: string; resolveDependencies?: boolean } & Partial<PartialNull<ModuleOptions>>
): Promise<ModuleInfo> {
const module = await this.fetchModule(
// TODO Lukas use correct assertions from resolvedId
this.getResolvedIdWithDefaults(resolvedId, EMPTY_OBJECT)!,
undefined,
false,
Expand Down Expand Up @@ -352,17 +352,24 @@ export class ModuleLoader {
}
}

// If this is a preload, then this method always waits for the dependencies of the module to be resolved.
// Otherwise if the module does not exist, it waits for the module and all its dependencies to be loaded.
// Otherwise it returns immediately.
// If this is a preload, then this method always waits for the dependencies of
// the module to be resolved.
// Otherwise, if the module does not exist, it waits for the module and all
// its dependencies to be loaded.
// Otherwise, it returns immediately.
private async fetchModule(
{ id, meta, moduleSideEffects, syntheticNamedExports }: ResolvedId,
{ assertions, id, meta, moduleSideEffects, syntheticNamedExports }: ResolvedId,
importer: string | undefined,
isEntry: boolean,
isPreload: PreloadType
): Promise<Module> {
const existingModule = this.modulesById.get(id);
if (existingModule instanceof Module) {
if (importer && doAssertionsDiffer(assertions, existingModule.info.assertions)) {
this.options.onwarn(
errInconsistentImportAssertions(existingModule.info.assertions, assertions, id, importer)
);
}
await this.handleExistingModule(existingModule, isEntry, isPreload);
return existingModule;
}
Expand All @@ -374,7 +381,8 @@ export class ModuleLoader {
isEntry,
moduleSideEffects,
syntheticNamedExports,
meta
meta,
assertions
);
this.modulesById.set(id, module);
this.graph.watchFiles[id] = true;
Expand Down Expand Up @@ -423,27 +431,30 @@ export class ModuleLoader {
importer: string,
resolvedId: ResolvedId
): Promise<Module | ExternalModule> {
// TODO Lukas handle internal
if (resolvedId.external) {
const { assertions, external, id, moduleSideEffects, meta } = resolvedId;
// TODO Lukas check assert for existing module
if (!this.modulesById.has(id)) {
this.modulesById.set(
let externalModule = this.modulesById.get(id);
if (!externalModule) {
externalModule = new ExternalModule(
this.options,
id,
new ExternalModule(
this.options,
id,
moduleSideEffects,
meta,
external !== 'absolute' && isAbsolute(id),
assertions
)
moduleSideEffects,
meta,
external !== 'absolute' && isAbsolute(id),
assertions
);
}

const externalModule = this.modulesById.get(id);
if (!(externalModule instanceof ExternalModule)) {
this.modulesById.set(id, externalModule);
} else if (!(externalModule instanceof ExternalModule)) {
return error(errInternalIdCannotBeExternal(source, importer));
} else if (doAssertionsDiffer(externalModule.info.assertions, assertions)) {
this.options.onwarn(
errInconsistentImportAssertions(
externalModule.info.assertions,
assertions,
source,
importer
)
);
}
return Promise.resolve(externalModule);
}
Expand Down Expand Up @@ -688,8 +699,21 @@ export class ModuleLoader {
);
}
if (resolution == null) {
// TODO Lukas handle existing resolved id conflicts
return (module.resolvedIds[specifier] ??= this.handleInvalidResolvedId(
const existingResolution = module.resolvedIds[specifier];
if (existingResolution) {
if (doAssertionsDiffer(existingResolution.assertions, assertions)) {
this.options.onwarn(
errInconsistentImportAssertions(
existingResolution.assertions,
assertions,
specifier,
importer
)
);
}
return existingResolution;
}
return (module.resolvedIds[specifier] = this.handleInvalidResolvedId(
await this.resolveId(specifier, module.id, EMPTY_OBJECT, false, assertions),
specifier,
module.id,
Expand Down
25 changes: 25 additions & 0 deletions src/utils/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@ const ADDON_ERROR = 'ADDON_ERROR',
FILE_NOT_FOUND = 'FILE_NOT_FOUND',
ILLEGAL_IDENTIFIER_AS_NAME = 'ILLEGAL_IDENTIFIER_AS_NAME',
ILLEGAL_REASSIGNMENT = 'ILLEGAL_REASSIGNMENT',
INCONSISTENT_IMPORT_ASSERTIONS = 'INCONSISTENT_IMPORT_ASSERTIONS',
INPUT_HOOK_IN_OUTPUT_PLUGIN = 'INPUT_HOOK_IN_OUTPUT_PLUGIN',
INVALID_CHUNK = 'INVALID_CHUNK',
INVALID_CONFIG_MODULE_FORMAT = 'INVALID_CONFIG_MODULE_FORMAT',
Expand Down Expand Up @@ -344,6 +345,30 @@ export function errIllegalImportReassignment(name: string, importingId: string):
};
}

export function errInconsistentImportAssertions(
existingAssertions: Record<string, string>,
newAssertions: Record<string, string>,
source: string,
importer: string
): RollupLog {
return {
code: INCONSISTENT_IMPORT_ASSERTIONS,
message: `Module "${relativeId(importer)}" tried to import "${relativeId(
source
)}" with ${formatAssertions(
newAssertions
)} assertions, but it was already imported elsewhere with ${formatAssertions(
existingAssertions
)} assertions. Please ensure that import assertions for the same module are always consistent.`
};
}

const formatAssertions = (assertions: Record<string, string>): string => {
const entries = Object.entries(assertions);
if (entries.length === 0) return 'no';
return entries.map(([key, value]) => `"${key}": "${value}"`).join(', ');
};

export function errInputHookInOutputPlugin(pluginName: string, hookName: string): RollupLog {
return {
code: INPUT_HOOK_IN_OUTPUT_PLUGIN,
Expand Down
11 changes: 11 additions & 0 deletions src/utils/parseAssertions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,3 +47,14 @@ export function getAssertionsFromImportExportDeclaration(
)
: EMPTY_OBJECT;
}

export function doAssertionsDiffer(
assertionA: Record<string, string>,
assertionB: Record<string, string>
): boolean {
const keysA = Object.keys(assertionA);
return (
keysA.length !== Object.keys(assertionB).length ||
keysA.some(key => assertionA[key] !== assertionB[key])
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
const path = require('path');
const ID_MAIN = path.join(__dirname, 'main.js');

module.exports = {
description: 'warns for conflicting import assertions',
options: {
external: id => id.startsWith('external')
},
warnings: [
{
code: 'INCONSISTENT_IMPORT_ASSERTIONS',
frame: `
1: import './other.js';
2: import 'external' assert { type: 'foo' };
3: import 'external' assert { type: 'bar' };
^
4: import 'external';
5: import('external', { assert: { type: 'baz' } });`,
id: ID_MAIN,
loc: {
column: 0,
file: ID_MAIN,
line: 3
},
message:
'Module "main.js" tried to import "external" with "type": "bar" assertions, but it was already imported elsewhere with "type": "foo" assertions. Please ensure that import assertions for the same module are always consistent.',
pos: 63
},
{
code: 'INCONSISTENT_IMPORT_ASSERTIONS',
frame: `
2: import 'external' assert { type: 'foo' };
3: import 'external' assert { type: 'bar' };
4: import 'external';
^
5: import('external', { assert: { type: 'baz' } });
6: import './dep.js' assert { type: 'foo' };`,
id: ID_MAIN,
loc: {
column: 0,
file: ID_MAIN,
line: 4
},
message:
'Module "main.js" tried to import "external" with no assertions, but it was already imported elsewhere with "type": "foo" assertions. Please ensure that import assertions for the same module are always consistent.',
pos: 105
},
{
code: 'INCONSISTENT_IMPORT_ASSERTIONS',
message:
'Module "main.js" tried to import "external" with "type": "baz" assertions, but it was already imported elsewhere with "type": "foo" assertions. Please ensure that import assertions for the same module are always consistent.'
},
{
code: 'INCONSISTENT_IMPORT_ASSERTIONS',
message:
'Module "other.js" tried to import "external" with "type": "quuz" assertions, but it was already imported elsewhere with "type": "foo" assertions. Please ensure that import assertions for the same module are always consistent.'
},
{
code: 'INCONSISTENT_IMPORT_ASSERTIONS',
message:
'Module "other.js" tried to import "dep.js" with "type": "bar" assertions, but it was already imported elsewhere with "type": "foo" assertions. Please ensure that import assertions for the same module are always consistent.'
}
]
};
Empty file.
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import './other.js';
import 'external' assert { type: 'foo' };
import 'external' assert { type: 'bar' };
import 'external';
import('external', { assert: { type: 'baz' } });
import './dep.js' assert { type: 'foo' };
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import 'external' assert { type: 'quuz' };
import './dep.js' assert { type: 'bar' };

0 comments on commit dc486f3

Please sign in to comment.