-
Notifications
You must be signed in to change notification settings - Fork 132
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(importStar): Cache non‑module results #80
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This could also be implemented such that you don't need to repeatedly call __getImportStarCache
using something like this:
__importStar = function (mod) {
var cache = typeof WeakMap === "function" ? new WeakMap() : null;
__importStar = function (mod) {
...
};
return _importStar(mod);
};
Which is similar to what we do for extendsStatics
.
19622b4
to
27c4b32
Compare
return result; | ||
} | ||
export var __importStar = function (mod) { | ||
var cache = typeof WeakMap === "function" ? new WeakMap() : null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you proposing this be implemented in TypeScript as well for inline helper emit? If so we'd need to introduce a checker error if you declare your own value named WeakMap
at the top of a module.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue there is that this is supposed to be an optimisation when two different modules that use tslib
import the same CommonJS module, the value returned by __importStar
is the same value for both imports, e.g.:
// modA.js
Object.defineProperty(exports, "__esModule", { value: true });
var tslib = require("tslib");
exports.example = tslib.__importStar(require("example"));
// modB.js
Object.defineProperty(exports, "__esModule", { value: true });
var tslib = require("tslib");
exports.example = tslib.__importStar(require("example"));
// test.js
var assert = require("assert");
var modA = require("./modA.js");
var modB = require("./modA.js");
// The following passes regardless of whether `example` has `__esModule` or not:
assert.strictEqual(modA.example, modB.example);
It doesn’t make sense to include the module cache in inline helpers, as modA
and modB
would then each have their own cache, and every new local __imporStar
call would then have a cache miss, since the cache wouldn’t be global.
export var __importStar = function (mod) { | ||
var cache = typeof WeakMap === "function" ? new WeakMap() : null; | ||
__importStar = function (mod) { | ||
if (mod === null || (typeof mod !== "object" && typeof mod !== "function")) return { default: mod }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Parens aren't needed here, since &&
has higher precedence than ||
.
if (mod === null || (typeof mod !== "object" && typeof mod !== "function")) return { default: mod }; | |
if (mod === null || typeof mod !== "object" && typeof mod !== "function") return { default: mod }; |
return result; | ||
var cache = typeof WeakMap === "function" ? new WeakMap() : null; | ||
__importStar = function (mod) { | ||
if (mod === null || (typeof mod !== "object" && typeof mod !== "function")) return { default: mod }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Parens aren't necessary here.
if (mod === null || (typeof mod !== "object" && typeof mod !== "function")) return { default: mod }; | |
if (mod === null || typeof mod !== "object" && typeof mod !== "function") return { default: mod }; | |
Babel has been doing this since babel/babel#10161, babel/babel#10574 and babel/babel#10585.