Skip to content

Commit

Permalink
Merge pull request #981 from huochunpeng/refactor-nodejs
Browse files Browse the repository at this point in the history
refactor(bundler): dry nodejs module resolving
  • Loading branch information
EisenbergEffect authored Jan 13, 2019
2 parents 7995b14 + a528b9a commit e96d143
Show file tree
Hide file tree
Showing 20 changed files with 229 additions and 216 deletions.
2 changes: 1 addition & 1 deletion lib/build/amodro-trace/read/cjs.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ module.exports = function cjs(fileName, fileContents, forceWrap) {
if (commonJsProps && (commonJsProps.dirname || commonJsProps.filename)) {
preamble = 'var __filename = module.uri || \'\', ' +
'__dirname = ' +
'__filename.substring(0, __filename.lastIndexOf(\'/\') + 1); ';
'__filename.slice(0, __filename.lastIndexOf(\'/\') + 1); ';
}

// Construct the wrapper boilerplate.
Expand Down
8 changes: 4 additions & 4 deletions lib/build/amodro-trace/write/replace.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,12 +24,12 @@ module.exports = function stubs(options) {
let dep = node.value;
// remove tailing '/'
if (dep.endsWith('/')) {
dep = dep.substr(0, dep.length - 1);
dep = dep.slice(0, -1);
}
// remove tailing '.js', but only when dep is not
// referencing a npm package main
if (dep.endsWith('.js') && !isPackageName(dep)) {
dep = dep.substr(0, dep.length - 3);
dep = dep.slice(0, -3);
}
// browser replacement;
if (replacement && replacement[dep]) {
Expand Down Expand Up @@ -74,9 +74,9 @@ module.exports = function stubs(options) {
};

function modify(contents, replacement) {
return contents.substr(0, replacement.start) +
return contents.slice(0, replacement.start) +
replacement.text +
contents.substr(replacement.end);
contents.slice(replacement.end);
}

function isPackageName(path) {
Expand Down
2 changes: 1 addition & 1 deletion lib/build/ast-matcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ function matchTerm(pattern) {
type = ARR;
}

if (type) return {type: type, name: possible.substr(6)};
if (type) return {type: type, name: possible.slice(6)};
}

/**
Expand Down
4 changes: 2 additions & 2 deletions lib/build/bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -291,7 +291,7 @@ exports.Bundle = class {
: null;

if (sourceMap !== null) {
let sourceRoot = parsedPath.dir.substring(process.cwd().length);
let sourceRoot = parsedPath.dir.slice(process.cwd().length);
sourceMap.sourceRoot = sourceRoot.replace(/\\/g, '\/');
}

Expand Down Expand Up @@ -359,7 +359,7 @@ exports.Bundle = class {
});
for (let i = 0; i < sortedFiles.length; ++i) {
let currentFile = sortedFiles[i];
sbuffer.push('> ' + (currentFile.contents.length / 1024).toFixed(2) + 'kb (' + ((currentFile.contents.length / contents.length) * 100).toFixed(2) + '%) - ' + (currentFile.path || currentFile.contents.substring(0, 200)));
sbuffer.push('> ' + (currentFile.contents.length / 1024).toFixed(2) + 'kb (' + ((currentFile.contents.length / contents.length) * 100).toFixed(2) + '%) - ' + (currentFile.path || currentFile.contents.slice(0, 200)));
if (currentFile.path) {
jsonRep[currentFile.path] = jsonRep[currentFile.path] || {};
jsonRep[currentFile.path].file = currentFile.path;
Expand Down
4 changes: 2 additions & 2 deletions lib/build/bundled-source.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ exports.BundledSource = class {

moduleId = moduleId.replace(/\\/g, '/');
if (moduleId.toLowerCase().endsWith('.js')) {
moduleId = moduleId.substr(0, moduleId.length - 3);
moduleId = moduleId.slice(0, -3);
}

this._moduleId = toDotDot(moduleId);
Expand Down Expand Up @@ -323,7 +323,7 @@ function possibleShortcut(moduleId, paths) {

if (_moduleId.startsWith(target + '/')) {
return {
fromId: toDotDot(key + _moduleId.substr(target.length)),
fromId: toDotDot(key + _moduleId.slice(target.length)),
toId: toDotDot(moduleId)
};
}
Expand Down
6 changes: 3 additions & 3 deletions lib/build/bundler.js
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ exports.Bundler = class {

if (id.endsWith('/index')) {
// if id is 'resources/index', shortId is 'resources'.
let shortId = id.substr(0, id.length - 6);
let shortId = id.slice(0, -6);
if (deps.delete(shortId)) {
// ok, someone try to use short name
bundle.addAlias(shortId, id);
Expand Down Expand Up @@ -272,7 +272,7 @@ exports.Bundler = class {
}

const nodeId = match[1];
const resourceId = match[2] && match[2].substr(1);
const resourceId = match[2] && match[2].slice(1);

const depInclusion = this.getDependencyInclusions().find(di => di.description.name === nodeId);

Expand Down Expand Up @@ -355,7 +355,7 @@ function ensurePathsRelativelyFromRoot(p) {
}
// trim off last '/'
if (p[key].endsWith('/')) {
p[key] = p[key].substr(0, p[key].length - 1);
p[key] = p[key].slice(0, -1);
}
}

Expand Down
2 changes: 1 addition & 1 deletion lib/build/dependency-description.js
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,7 @@ function filePathToModuleId(filePath) {
let moduleId = path.normalize(filePath).replace(/\\/g, '/');

if (moduleId.toLowerCase().endsWith('.js')) {
moduleId = moduleId.substr(0, moduleId.length - 3);
moduleId = moduleId.slice(0, -3);
}

return moduleId;
Expand Down
95 changes: 6 additions & 89 deletions lib/build/dependency-inclusion.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
const path = require('path');
const SourceInclusion = require('./source-inclusion').SourceInclusion;
const minimatch = require('minimatch');
const fs = require('../file-system');
const Utils = require('./utils');
const logger = require('aurelia-logging').getLogger('DependencyInclusion');

const knownNonJsExtensions = ['.json', '.css', '.svg', '.html'];
Expand Down Expand Up @@ -66,11 +66,11 @@ exports.DependencyInclusion = class {
return Promise.resolve();
}

if (removeJsExtension(resolved) !== removeJsExtension(resource)) {
if (Utils.removeJsExtension(resolved) !== Utils.removeJsExtension(resource)) {
// alias bootstrap/css/bootstrap.css to bootstrap/lib/css/bootstrap.css
this.bundle.addAlias(
this.description.name + '/' + removeJsExtension(resource),
this.description.name + '/' + removeJsExtension(resolved)
this.description.name + '/' + Utils.removeJsExtension(resource),
this.description.name + '/' + Utils.removeJsExtension(resolved)
);
}

Expand Down Expand Up @@ -119,7 +119,7 @@ exports.DependencyInclusion = class {
ids.forEach(id => {
// for aurelia-templating-resources/dist/commonjs/if
// compact name is aurelia-templating-resources/if
let compactResource = id.substr(commonLen);
let compactResource = id.slice(commonLen);
if (compactResource) {
let compactId = this.description.name + '/' + compactResource;
aliases[compactId] = id;
Expand Down Expand Up @@ -176,18 +176,10 @@ function resolvedResource(resource, description, projectRoot) {

function validResource(resource, base) {
const resourcePath = path.resolve(base, resource);
const loaded = nodejsLoadAsFile(resourcePath) || nodejsLoadAsDirectory(resourcePath);
const loaded = Utils.nodejsLoad(resourcePath);
if (loaded) return path.relative(base, loaded).replace(/\\/g, '/');
}

function removeJsExtension(filePath) {
if (path.extname(filePath).toLowerCase() === '.js') {
return filePath.substr(0, filePath.length - 3);
}

return filePath;
}

function commonLength(ids) {
let parts = ids[0].split('/');
let rest = ids.slice(1);
Expand All @@ -206,78 +198,3 @@ function commonLength(ids) {

return common.length;
}

// TODO: refactor
// Extract this, plus part of package-analyzer into nodejs-utils.
// I will do it after 2018-11-15 after my vacation. Chunpeng Huo

// https://nodejs.org/dist/latest-v8.x/docs/api/modules.html
// after "high-level algorithm in pseudocode of what require.resolve() does"
function nodejsLoadAsFile(resourcePath) {
if (fs.isFile(resourcePath)) {
return resourcePath;
}

if (fs.isFile(resourcePath + '.js')) {
return resourcePath + '.js';
}

if (fs.isFile(resourcePath + '.json')) {
return resourcePath + '.json';
}
// skip .node file that nobody uses
}

function nodejsLoadIndex(resourcePath) {
if (!fs.isDirectory(resourcePath)) return;

const indexJs = path.join(resourcePath, 'index.js');
if (fs.isFile(indexJs)) {
return indexJs;
}

const indexJson = path.join(resourcePath, 'index.json');
if (fs.isFile(indexJson)) {
return indexJson;
}
// skip index.node file that nobody uses
}

function nodejsLoadAsDirectory(resourcePath) {
if (!fs.isDirectory(resourcePath)) return;

const packageJson = path.join(resourcePath, 'package.json');

if (fs.isFile(packageJson)) {
let metadata;
try {
metadata = JSON.parse(fs.readFileSync(packageJson));
} catch (err) {
logger.error(err);
return;
}
let metaMain;
// try 1.browser > 2.module > 3.main
// the order is to target browser.
// when aurelia-cli introduces multi-targets build,
// it probably should use different order for electron app
// for electron 1.module > 2.browser > 3.main
if (typeof metadata.browser === 'string') {
// use package.json browser field if possible.
metaMain = metadata.browser;
} else if (typeof metadata.module === 'string') {
// prefer es module format over cjs, just like webpack.
// this improves compatibility with TypeScript.
metaMain = metadata.module;
} else if (typeof metadata.main === 'string') {
metaMain = metadata.main;
}

let mainFile = metaMain || 'index';
const mainResourcePath = path.resolve(resourcePath, mainFile);
return nodejsLoadAsFile(mainResourcePath) || nodejsLoadIndex(mainResourcePath);
}

return nodejsLoadIndex(resourcePath);
}

4 changes: 2 additions & 2 deletions lib/build/find-deps.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ const auJsDepFinder = jsDepFinder(
// even if we need to support it,
// I can go down to astMatcher to support it, no sweat.
// 1. ignores deps if base url starts with https:// or http://
// 2. use path.resolve('/', baseUrl, dep).substr(1) to get real deps
// 2. use path.resolve('/', baseUrl, dep).slice(1) to get real deps
);

const _checkConfigureFunc = [
Expand Down Expand Up @@ -196,7 +196,7 @@ function _add(deps) {

let clean = d.trim();
// strip off leading /
if (clean[0] === '/') clean = clean.substr(1);
if (clean[0] === '/') clean = clean.slice(1);

// There is some npm package call itself like "popper.js",
// cannot strip .js from it.
Expand Down
Loading

0 comments on commit e96d143

Please sign in to comment.