Skip to content

Commit

Permalink
Use module.export(getters, true) for const ExportNamedDeclaration.
Browse files Browse the repository at this point in the history
These changes let the runtime know that the values returned by the getter
functions for

  export const a = 1, b = 2;

will never change. Unfortunately, it's more difficult to detect that the
same is true for

  const a = 1, b = 2;
  export { a, b };

though that's an area for future improvement.

Part of #134.
  • Loading branch information
benjamn committed May 26, 2017
1 parent 2f8c2db commit 42a9f48
Show file tree
Hide file tree
Showing 4 changed files with 50 additions and 14 deletions.
3 changes: 2 additions & 1 deletion lib/entry.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ Entry.getOrCreate = function (exported) {
Ep.addGetters = function (getters, constant) {
var names = Object.keys(getters);
var nameCount = names.length;
constant = !! constant;

for (var i = 0; i < nameCount; ++i) {
var name = names[i];
Expand All @@ -81,7 +82,7 @@ Ep.addGetters = function (getters, constant) {
// Should this throw if this.getters[name] exists?
! (name in this.getters)) {
this.getters[name] = getter;
getter.constant = !! constant;
getter.constant = constant;
}
}
};
Expand Down
57 changes: 46 additions & 11 deletions lib/import-export-visitor.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,15 @@ class ImportExportVisitor extends Visitor {
codeToInsert += '"use strict";';
}

const namedExports = toModuleExport(
this,
bodyInfo.hoistedExportsMap
);
const addExportsMap = (map, constant) => {
const namedExports = toModuleExport(this, map, constant);
if (namedExports) {
codeToInsert += namedExports;
}
};

if (namedExports) {
codeToInsert += namedExports;
}
addExportsMap(bodyInfo.hoistedExportsMap, false);
addExportsMap(bodyInfo.hoistedConstExportsMap, true);

if (bodyInfo.hoistedExportsString) {
codeToInsert += bodyInfo.hoistedExportsString;
Expand Down Expand Up @@ -292,7 +293,14 @@ class ImportExportVisitor extends Visitor {
}

hoistExports(this, path, specifierMap, "declaration");
addExportedLocalNames(this, specifierMap);

if (canExportedValuesChange(decl)) {
// We can skip adding declared names to this.exportedLocalNames if
// the declaration was a const-kinded VariableDeclaration, because
// the assignmentVisitor will not need to worry about changes to
// these variables.
addExportedLocalNames(this, specifierMap);
}

return;
}
Expand Down Expand Up @@ -517,6 +525,7 @@ function getBlockBodyInfo(visitor, path) {
bodyInfo.insertCharIndex = insertCharIndex;
bodyInfo.insertNodeIndex = insertNodeIndex;
bodyInfo.hoistedExportsMap = Object.create(null);
bodyInfo.hoistedConstExportsMap = Object.create(null);
bodyInfo.hoistedExportsString = "";
bodyInfo.hoistedImportsString = "";

Expand Down Expand Up @@ -564,6 +573,7 @@ function hoistExports(visitor, exportDeclPath, mapOrString, childName) {
}

const bodyInfo = getBlockBodyInfo(visitor, exportDeclPath);
const constant = ! canExportedValuesChange(exportDeclPath.getValue());

if (typeof mapOrString !== "string") {
const keys = Object.keys(mapOrString);
Expand All @@ -576,7 +586,9 @@ function hoistExports(visitor, exportDeclPath, mapOrString, childName) {

for (let j = 0; j < localCount; ++j) {
addToSpecifierMap(
bodyInfo.hoistedExportsMap,
constant
? bodyInfo.hoistedConstExportsMap
: bodyInfo.hoistedExportsMap,
exported,
locals[j]
);
Expand All @@ -590,6 +602,27 @@ function hoistExports(visitor, exportDeclPath, mapOrString, childName) {
visitor.madeChanges = true;
}

function canExportedValuesChange(exportDecl) {
if (exportDecl) {
if (exportDecl.type === "ExportDefaultDeclaration") {
const dd = exportDecl.declaration;
return (dd.type === "FunctionDeclaration" ||
dd.type === "ClassDeclaration");
}

if (exportDecl.type === "ExportNamedDeclaration") {
const dd = exportDecl.declaration;
if (dd &&
dd.type === "VariableDeclaration" &&
dd.kind === "const") {
return false;
}
}
}

return true;
}

function makeUniqueKey(visitor) {
return visitor.nextKey++;
}
Expand Down Expand Up @@ -791,7 +824,7 @@ function toModuleImport(source, specifierMap, uniqueKey) {
return code;
}

function toModuleExport(visitor, specifierMap) {
function toModuleExport(visitor, specifierMap, constant) {
let code = "";
const exportedKeys = Object.keys(specifierMap);
const keyCount = exportedKeys.length;
Expand Down Expand Up @@ -823,7 +856,9 @@ function toModuleExport(visitor, specifierMap) {
}
}

code += "});";
// The second argument to module.export indicates whether the getter
// functions provided in the first argument are constant or not.
code += constant ? "},true);" : "});";

return code;
}
Expand Down
2 changes: 1 addition & 1 deletion test/output/declarations-basic/expected.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"use strict";module.export({a:()=>a,b:()=>b,c:()=>c,d:()=>d});const a = 1;
"use strict";module.export({c:()=>c,d:()=>d});module.export({a:()=>a,b:()=>b},true);const a = 1;
const b = function () {
return d;
};
Expand Down
2 changes: 1 addition & 1 deletion test/output/name/expected.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
"use strict";module.export({id:()=>id,name:()=>name,foo:()=>foo});const path = require("path");
"use strict";module.export({foo:()=>foo});module.export({id:()=>id,name:()=>name},true);const path = require("path");

const id = module.id,
name = path.basename(__filename);
Expand Down

0 comments on commit 42a9f48

Please sign in to comment.