Don't warn about unresolved modules in Browserify/Webpack bundle files.

Fixes https://github.com/practicalmeteor/meteor-mocha/issues/19
Related: https://github.com/meteor/todos/issues/90
See also: 62c41b2ef5
This commit is contained in:
Ben Newman
2016-03-29 17:33:06 -04:00
parent f6fb8b6114
commit a86c61eb9b
4 changed files with 131 additions and 62 deletions

View File

@@ -57,7 +57,7 @@ import { isTestFilePath } from './test-files.js';
// Cache the (slightly post-processed) results of linker.fullLink.
const CACHE_SIZE = process.env.METEOR_LINKER_CACHE_SIZE || 1024*1024*100;
const CACHE_DEBUG = !! process.env.METEOR_TEST_PRINT_LINKER_CACHE_DEBUG;
const LINKER_CACHE_SALT = 2; // Increment this number to force relinking.
const LINKER_CACHE_SALT = 3; // Increment this number to force relinking.
const LINKER_CACHE = new LRU({
max: CACHE_SIZE,
// Cache is measured in bytes. We don't care about servePath.
@@ -766,7 +766,7 @@ export class PackageSourceBatch {
function handleMissing(missingNodeModules) {
const missingMap = new Map;
Object.keys(missingNodeModules).forEach(id => {
_.each(missingNodeModules, (info, id) => {
const parts = id.split("/");
let name = null;
@@ -784,10 +784,16 @@ export class PackageSourceBatch {
return;
}
if (missingMap.has(name)) {
missingMap.get(name).push(id);
} else {
missingMap.set(name, [id]);
if (! missingMap.has(name)) {
missingMap.set(name, {});
}
const missing = missingMap.get(name);
if (! _.has(missing, id) ||
! info.possiblySpurious) {
// Allow any non-spurious identifier to replace an existing
// possibly spurious identifier.
missing[id] = info;
}
});
@@ -807,7 +813,7 @@ export class PackageSourceBatch {
handleMissing(allMissingNodeModules);
_.each(allRelocatedNodeModules, (packageName, id) => {
_.each(allRelocatedNodeModules, (info, id) => {
delete allMissingNodeModules[id];
});
@@ -855,6 +861,13 @@ export class PackageSourceBatch {
const warnings = [];
_.each(missingNodeModules, (info, id) => {
if (info.possiblySpurious) {
// Silence warnings for missing dependencies in Browserify/Webpack
// bundles, since we can reasonably conclude at this point that
// they are false positives.
return;
}
if (info.packageName === "avital:mocha") {
// Silence warnings for the avital:mocha package, because we know
// they're spurious, and it seems poor form to spew a bunch of

View File

@@ -1,5 +1,7 @@
import assert from "assert";
import {isString, has, keys, each, map, without} from "underscore";
import {
isString, isEmpty, has, keys, each, map, without
} from "underscore";
import {sha1, readAndWatchFileWithHash} from "../fs/watch.js";
import {matches as archMatches} from "../utils/archinfo.js";
import {findImportedModuleIdentifiers} from "./js-analyze.js";
@@ -65,9 +67,9 @@ const defaultExtensionHandlers = {
// of ImportScanner (which do not persist across builds).
const IMPORT_SCANNER_CACHE = new LRU({
max: 1024*1024,
length(value) {
length(ids) {
let total = 40; // size of key
value.forEach(str => { total += str.length; });
each(ids, (info, id) => { total += id.length; });
return total;
}
});
@@ -199,49 +201,46 @@ export default class ImportScanner {
}
addNodeModules(identifiers) {
assert.ok(identifiers);
assert.ok(typeof identifiers === "object");
assert.ok(! Array.isArray(identifiers));
const newlyMissing = Object.create(null);
const newlyAdded = Object.create(null);
if (identifiers) {
if (typeof identifiers === "object" &&
! Array.isArray(identifiers)) {
identifiers = Object.keys(identifiers);
}
if (! isEmpty(identifiers)) {
const previousAllMissingNodeModules = this.allMissingNodeModules;
this.allMissingNodeModules = newlyMissing;
if (identifiers.length > 0) {
const previousAllMissingNodeModules = this.allMissingNodeModules;
this.allMissingNodeModules = newlyMissing;
try {
this._scanFile({
sourcePath: "fake.js",
// By specifying the .deps property of this fake file ahead of
// time, we can avoid calling findImportedModuleIdentifiers in the
// _scanFile method.
deps: identifiers,
});
try {
this._scanFile({
sourcePath: "fake.js",
// By specifying the .deps property of this fake file ahead of
// time, we can avoid calling findImportedModuleIdentifiers in the
// _scanFile method.
deps: identifiers,
});
} finally {
this.allMissingNodeModules = previousAllMissingNodeModules;
} finally {
this.allMissingNodeModules = previousAllMissingNodeModules;
each(identifiers, (info, id) => {
if (! has(newlyMissing, id)) {
newlyAdded[id] = info;
}
});
identifiers.forEach(id => {
if (! has(newlyMissing, id)) {
newlyAdded[id] = this.name;
}
});
// Remove previously seen missing module identifiers from
// newlyMissing and merge the new identifiers back into
// this.allMissingNodeModules.
each(keys(newlyMissing), key => {
if (has(previousAllMissingNodeModules, key)) {
delete newlyMissing[key];
} else {
previousAllMissingNodeModules[key] =
newlyMissing[key];
}
});
}
// Remove previously seen missing module identifiers from
// newlyMissing and merge the new identifiers back into
// this.allMissingNodeModules.
each(keys(newlyMissing), key => {
if (has(previousAllMissingNodeModules, key)) {
delete newlyMissing[key];
} else {
previousAllMissingNodeModules[key] =
newlyMissing[key];
}
});
}
}
@@ -302,10 +301,10 @@ export default class ImportScanner {
return IMPORT_SCANNER_CACHE.get(file.hash);
}
const result = keys(findImportedModuleIdentifiers(
const result = findImportedModuleIdentifiers(
file.dataString,
file.hash,
));
);
// there should always be file.hash, but better safe than sorry
if (file.hash) {
@@ -332,7 +331,7 @@ export default class ImportScanner {
throw e;
}
each(file.deps, id => {
each(file.deps, (info, id) => {
const resolved = this._tryToResolveImportedPath(file, id);
if (! resolved) {
return;
@@ -702,8 +701,12 @@ export default class ImportScanner {
const isApp = ! this.name;
if (isApp && isNative && archMatches(this.bundleArch, "web")) {
// To ensure the native module can be evaluated at runtime,
// register a dependency on meteor-node-stubs/defs/<id>.js.
return this._resolveNodeModule(file, nativeModulesMap[id]);
// register a dependency on meteor-node-stubs/deps/<id>.js.
const stubsId = nativeModulesMap[id];
if (isString(stubsId) && stubsId !== id) {
file.deps[stubsId] = file.deps[id];
return this._resolveNodeModule(file, stubsId);
}
}
// If the imported identifier is neither absolute nor relative, but
@@ -712,11 +715,24 @@ export default class ImportScanner {
// missing dependency so that we can include it in the app bundle.
const missing = file.missingNodeModules || Object.create(null);
file.missingNodeModules = missing;
this.allMissingNodeModules[id] = missing[id] = {
const possiblySpurious =
has(file.deps, id) &&
file.deps[id].possiblySpurious;
const info = missing[id] = {
packageName: this.name,
parentPath: absPath,
bundleArch: this.bundleArch,
possiblySpurious,
};
if (! has(this.allMissingNodeModules, id) ||
! info.possiblySpurious) {
// Allow any non-spurious identifier to replace an existing
// possibly spurious identifier.
this.allMissingNodeModules[id] = info;
}
}
// If the dependency is still not resolved, it might be handled by the
@@ -805,7 +821,7 @@ export default class ImportScanner {
const pkgFile = {
type: "js", // We represent the JSON module with JS.
data,
deps: [], // Avoid accidentally re-scanning this file.
deps: {}, // Avoid accidentally re-scanning this file.
sourcePath: relPkgJsonPath,
installPath: this._getInstallPath(pkgJsonPath),
servePath: relPkgJsonPath,

View File

@@ -2,6 +2,8 @@ import { parse } from 'meteor-babel';
import { analyze as analyzeScope } from 'escope';
import LRU from "lru-cache";
const hasOwn = Object.prototype.hasOwnProperty;
var AST_CACHE = new LRU({
max: Math.pow(2, 12),
length(ast) {
@@ -65,13 +67,13 @@ export function findImportedModuleIdentifiers(source, hash) {
const ast = tryToParse(source, hash);
function walk(node, left, right) {
function walk(node, left, right, requireIsBound) {
if (left >= right) {
// The window of possible indexes is empty, so we can ignore
// the entire subtree rooted at this node.
} else if (Array.isArray(node)) {
for (var i = 0, len = node.length; i < len; ++i) {
walk(node[i], left, right);
walk(node[i], left, right, requireIsBound);
}
} else if (isNode(node)) {
const start = node.start;
@@ -83,16 +85,19 @@ export function findImportedModuleIdentifiers(source, hash) {
while (left < right && end < possibleIndexes[right - 1]) --right;
if (left < right) {
if (! requireIsBound &&
isFunctionWithParameter(node, "require")) {
requireIsBound = true;
}
let id = getRequiredModuleId(node);
if (typeof id === "string") {
identifiers[id] = node;
return;
return addIdentifier(id, "require", requireIsBound);
}
id = getImportedModuleId(node);
if (typeof id === "string") {
identifiers[id] = node;
return;
return addIdentifier(id, "import", requireIsBound);
}
// Continue traversing the children of this node.
@@ -106,13 +111,30 @@ export function findImportedModuleIdentifiers(source, hash) {
continue;
}
walk(node[key], left, right);
walk(node[key], left, right, requireIsBound);
}
}
}
}
walk(ast, 0, possibleIndexes.length);
function addIdentifier(id, type, requireIsBound) {
const entry = hasOwn.call(identifiers, id)
? identifiers[id]
: identifiers[id] = { possiblySpurious: true };
if (type === "require") {
// If the identifier comes from a require call, but require is not a
// free variable, then this dependency might be spurious.
entry.possiblySpurious =
entry.possiblySpurious && requireIsBound;
} else {
// The import keyword can't be shadowed, so any dependencies
// registered by import statements should be trusted absolutely.
entry.possiblySpurious = false;
}
}
walk(ast, 0, possibleIndexes.length, false);
return identifiers;
}
@@ -125,6 +147,18 @@ function isNode(value) {
&& typeof value.end === "number";
}
function isFunctionWithParameter(node, name) {
if (node.type === "FunctionExpression" ||
node.type === "FunctionDeclaration" ||
node.type === "ArrowFunctionExpression") {
return node.params.some(
param =>
param.type === "Identifier" &&
param.name === name
);
}
}
function getRequiredModuleId(node) {
if (node.type === "CallExpression" &&
node.callee.type === "Identifier" &&

View File

@@ -423,7 +423,13 @@ var File = function (inputFile, module) {
self.servePath = inputFile.servePath;
// Module identifiers imported or required by this module, if any.
self.deps = inputFile.deps || [];
if (Array.isArray(inputFile.deps)) {
self.deps = inputFile.deps;
} else if (inputFile.deps && typeof inputFile.deps === "object") {
self.deps = Object.keys(inputFile.deps);
} else {
self.deps = [];
}
// True if the input file should not be evaluated eagerly.
self.lazy = inputFile.lazy; // could be `true`, `false` or `undefined` <sigh>