-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
Invalidate cache when using import #49442
Comments
the import cache is purposely unexposed. adding a query has been the generally accepted ecosystem practice to re-import something. however, a failure to import something will not fill the cache. this trivial program works fine for me (assuming import fs from 'fs';
import('./nope.mjs')
.catch(() => fs.writeFileSync('./nope.mjs'))
.then(() => import('./nope.mjs'))
.then(console.log); |
@devsnek, hmm, might this be limited to imports that use node_modules? This similarly trivial program fails for me the first time, but not the second. import child_process from 'child_process';
import('color-names')
.catch(() => child_process.execSync('npm install --no-save color-names'))
.then(() => import('color-names'))
.then(console.log); |
Note that the JS spec requires imports to be deterministic/idempotent on a
source text. Exposure of a cache would not allow you to fix the code above.
…On Fri, Apr 5, 2019, 12:01 PM Gus Caplan ***@***.***> wrote:
the import cache is purposely unexposed. adding a query has been the
generally accepted ecosystem practice to re-import something.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#49442>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/AAOUo5N5tioTq2s_t431OrihH_wH8Qagks5vd4FSgaJpZM4cfV4g>
.
|
if its just happening with node_modules it could be #26926 |
can this be closd? |
I think a use case like this would hopefully be implemented as a loader. Do we already track this as a use case in that context? |
@jkrems we have old documents with that as a feature, but no success criteria examples. |
FYI, I'm implementing ESM support in Mocha (mochajs/mocha#4038), and cannot currently implement "watch mode", whereby Mocha watches the test files, and reruns them when they change. So "watch mode" in Mocha, in the first iteration, will probably not support ESM, which is a bummer. While we could use cache busting query parameters, that would mean that we are always increasing memory usage, and old and never-to-be-used versions of the file will continue staying in memory due to the cache holding on to them. And I'm not sure a loader would help here, as the loader also has no access to the cache. |
An API for unloading modules certainly makes sense.
Usually with a direct registry API there is the tracing issue. An API that
handles dependency removal can be useful.
A simple API might be something like -
import { unload } from ‘module’;
unload(import.meta.url); // returns true
Where the unload function would remove that module including all its
dependencies from the registry. If in a cycle the whole cycle would be
removed.
A subsequent module load would refresh all the loads anew.
Other problems to ensure work out is what if modules in the tree are still
in-progress. I’d be tempted to say it should fail for that case and only
work when all modules have either errored or completed.
We still have memory leak concerns as v8 doesn’t lend itself easily to
module GC still. But Node.js can lead the way here as it should. It will be
an ongoing process to get there, but the API can come first.
The main questions then seem to be:
* are we ok exposing this as a module or should it be tied to loaders? If
tied to loaders how would userland code request this? Or don’t we want it
to - as in Mocha should run a new context with a loader?
* should it be a direct registry API (get/set) with tracing, or should it
be a deep API like the example above
* finally, ironing out the partially loaded tree edge cases
…On Tue, Nov 26, 2019 at 00:09 Gil Tayar ***@***.***> wrote:
FYI, I'm implementing ESM support in Mocha (mochajs/mocha#4038
<mochajs/mocha#4038>), and cannot currently
implement "watch mode", whereby Mocha watches the test files, and reruns
them when they change. So "watch mode" in Mocha, in the first iteration,
will probably not support ESM, which is a bummer.
While we *could* use cache busting query parameters, that would mean that
we are always increasing memory usage, and old and never-to-be-used
versions of the file will continue staying in memory due to the cache
holding on to them.
And I'm not sure a loader would help here, as the loader also has no
access to the cache.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#49442?email_source=notifications&email_token=AAESFSTBSWVLXPTWDYZOHSDQVSVQRA5CNFSM4HD5LYQKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEFEXNDI#issuecomment-558462605>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAESFSWTJXGB4J6WWNMOLRDQVSVQRANCNFSM4HD5LYQA>
.
|
I'm really not a fan of the idea of our module cache being anything except insert-only. CJS cache modification is bad already, and CJS modules don't even form graphs. Additionally, other runtimes (like browsers) will never expose this functionality, so some alternative system will have to be used for them regardless of what node does, in which case it seems like that system could just be used for node. |
@giltayar have you looked into using Workers or other solutions to have a module cache that you can destroy (such as by killing the Worker)? |
@bmeck - interesting. That would mean that the tests themselves run in Workers. While I am theoretically familiar with workers, I haven't yet had any experience with them: is any code that runs in the main process compatible with worker inside a worker? In other words, compatibility-wise, would all test code that works today in the "main process" work inside workers? I wouldn't want Mocha to have a version (even a semver-major breaking one) where developers will need to tweak their code because now it's running inside a worker. I'm guessing that there's a vast amount of that code running inside Mocha, and any incompatibility would be a deal breaker. |
there are differences between workers and the main thread, mostly surrounding the functions on |
Looking at the list, I can see I would hesitate to say this, as my only contribution to Mocha currently is this pull request, but I would guess that the owners would veto this. Or maybe allow this only if we add a |
If it wasn't apparent from the above, I believe I would still prefer a "module unloading" API, unless the working group is adamant and official about not having one, of course. Which would probably mean going the "subprocess"/"worker" route. |
i admittedly don't know much about mocha... is using a separate process not doable either? |
I'll go back to the Mocha contributors team with this. |
Hi, I work on Mocha!! I am trying to see how we can move @giltayar's PR forward. There are actually two situations in which "module unloading" is needed in Mocha:
In the first case, it's possible, though probably at a performance cost, Mocha could leverage workers to handle ESM. I don't know enough about workers to say whether this will provide a sufficient environment for the test cases, but it feels like a misuse of the workers feature. At minimum it seems like a lot of added complexity. In the second case, I can't see how using workers would be feasible. Test authors need to be able to mock modules on-the-fly and reference them directly from test cases, using mocking frameworks. I don't know why this sort of behavior was omitted from the official specification. If the reasons involve "browser security", well, it further reinforces that browsers are a hostile environment for testing. I do know that this behavior is a very real need for many, from library and tooling authors down to developers working on production code. We do need an "unload module" API; until such a thing lands, tools will be limited, implementations will be difficult (if possible), and end users will be frustrated when their tests written in ESM don't work. I will also be frustrated, because those frustrated users will complain in Mocha's issue tracker! I'm happy to talk in further detail about use-cases, but I'm eager to put an eventual API description in the more-capable hands of people like @guybedford. @devsnek Given that enabling it also enables tooling, I'm curious why you feel locking this sort of thing down is a better direction? cc @nodejs/tooling P.S. I will be at the collab summit, and the tooling group will be hosting a collaboration session; maybe this can be a topic of discussion, or vice-versa if there's a modules group meeting...? |
Do you need unloading API for the watch mode? Yes, you need it to update the changed module code. However, it is enough to handle watch mode? No, as long as the idea is to use changed module, as you have to find So - an ability to invalidate a cache line is not enough, for the mocking task we also have to know the cache graph, we could traverse and understand which work should be done.
In my opinion - this feature is something missing for a proper code splitting. There are already 100Mb bundles, separated into hundreds of pieces, you will never load simultaneously. But you if will - there is no way to unload them. Eventually, the Page or an Application would just crash. |
@boneskull - the second case you mentioned, I believe can and should be handled by module "loaders", which are a formal way to do "require hooks" for ESM. These will enable testing frameworks (like sinon and others) to manipulate how ES modules are loaded, and, for example, exchange other modules for theirs. The spec and implementation for that are actively being discussed and worked on by the modules working group (see nodejs/modules#351). |
I also need this. I'm making a template rendering engine. When generating the compiled template, I read from a custom format and output to a .js file (a standard ES Module). In order to use the file, I just import it. Upon file changes, I would like to re-write the file, clear the import cache and then re-import it. |
These all sound like use cases for V8's LiveEdit debug api (https://chromedevtools.github.io/devtools-protocol/v8/Debugger#method-setScriptSource). You can call into it using https://nodejs.org/api/inspector.html. cc @giltayar @boneskull |
+1 for unloading ES Modules. |
@devsnek Can you provide a little example or pseudo-code on usage of setScriptSource. I have been researching for an 1hour without progress. Thanks |
@devsnek ok I progressed, I will post my findings back |
@georges-gomes you can subscribe to the |
@georges-gomes If you are successful, I would be very grateful if you could post a short description on how you could use |
I mean, the while point of the module customization hooks is to allow the user to violate spec however they please (that Node can achieve). It's not like importing CoffeeScript is spec compliant. If we're worried about dependencies using this API to mischievous ends we could gate it behind a flag or create a permission for it; but I'm not sure what the risk is. |
It is, though. CoffeeScript would be a Cyclic Module Record (a module which participates in the specified cyclic resolution algorithm). Cyclic Module Record is described in the specification as "abstract". Source Text Module Record, which is the ES Modules that we all know, is a concrete implementation of that abstract interface. So a CoffeeScript module would be a separate but valid implementation of a Cyclic Module Record. If you go up one level there is a plain Module Record, from which Cyclic Module Record is implemented. This provides the means for modules which don't participate in the cyclic resolution algorithm (wasm, json, or the whole Anyway, you are right that the loaders API provides the means to break specification, since the result of
The risks are impossible to enumerate since we're reneging on an presumed invariant. Like what happens if a user attempts to evict Anyway the risks are probably mostly hypothetical in nature. I'll try and open a PR soon to continue the discussion. |
I'm abandoning the PR at #50618. After more reflection I think this is going to harm the ecosystem more than it will do any good. From what I can tell there are 3-4 different use-cases mentioned in this issue which can be solved in other ways: Q: "How do I retry a module whose file content was created after a failed import" Q: "How do I reload a module and all its dependencies" Q: "How do I add module support to unit test frameworks" Q: "How do I hot reload modules during development" |
@laverdet Does this look fine to you (this is a reworked version of #50618 (comment))? |
@pygy The loader code looks correct and does what it says it does. import * as mDev from './my-module.js?dev'
process.env.NODE_ENV='production'
import * as mProd from './my-module.js?prod' This counter-example from the documentation doesn't make sense though. Imports are not executed in an imperative manner, so the body of |
Good catch, thanks this should have been dynamic imports. Fixed, and I added an example with dependencies for extra clarity: With dependenciesSuppose these files: // foo.js
export {x} from "./bar.js"
// bar.js
export const x = {} We can then do import "esm-reload"
const foo1 = await import("./foo.js?instance=1")
const bar1 = await import("./bar.js?instance=1")
const foo2 = await import("./foo.js?instance=2")
const bar2 = await import("./bar.js?instance=2")
assert.equal(foo1.x, bar1.x)
assert.equal(foo2.x, bar1.x)
assert.notEqual(bar1.x, bar2.x) Edit again: https://www.npmjs.com/package/esm-reload |
There has been no activity on this feature request for 5 months. To help maintain relevant open issues, please add the
never-stale
|
Should probably close this as "wont do"? |
My example import fs from 'node:fs';
import { isBuiltin } from 'node:module';
import path from 'node:path';
import { fileURLToPath } from 'node:url';
import {
createContext,
type Module,
type ModuleLinker,
SourceTextModule,
SyntheticModule
} from 'node:vm';
const ROOT_MODULE = '__root_module__';
const link: ModuleLinker = async (specifier: string, referrer: Module) => {
// Node.js native module
const isNative = isBuiltin(specifier);
// node_modules
const isNodeModules =
!isNative && !specifier.startsWith('./') && !specifier.startsWith('/');
if (isNative || isNodeModules) {
const nodeModule = await import(specifier);
const keys = Object.keys(nodeModule);
const module = new SyntheticModule(
keys,
function () {
keys.forEach((key) => {
this.setExport(key, nodeModule[key]);
});
},
{
identifier: specifier,
context: referrer.context
}
);
await module.link(link);
await module.evaluate();
return module;
} else {
const dir =
referrer.identifier === ROOT_MODULE
? import.meta.dirname
: path.dirname(referrer.identifier);
const filename = path.resolve(dir, specifier);
const text = fs.readFileSync(filename, 'utf-8');
const module = new SourceTextModule(text, {
initializeImportMeta,
identifier: specifier,
context: referrer.context,
// @ts-expect-error
importModuleDynamically: link
});
await module.link(link);
await module.evaluate();
return module;
}
};
export async function importEsm(identifier: string): Promise<any> {
const context = createContext({
console,
process,
[ROOT_MODULE]: {}
});
const module = new SourceTextModule(
`import * as root from '${identifier}';
${ROOT_MODULE} = root;`,
{
identifier: ROOT_MODULE,
context
}
);
await module.link(link);
await module.evaluate();
return context[ROOT_MODULE];
}
function initializeImportMeta(meta: ImportMeta, module: SourceTextModule) {
meta.filename = import.meta.resolve(module.identifier, import.meta.url);
meta.dirname = path.dirname(meta.filename);
meta.resolve = import.meta.resolve;
meta.url = fileURLToPath(meta.filename);
} Use it const module = await importEsm('filename'); |
If you are interested, we created https://adonisjs.com/blog/hmr-in-adonisjs |
Hi, the ability to invalidate cache when using https://nodejs.org/api/test.html#mockmodulespecifier-options Here is a use-case, where you can run into a trouble. fs-extended.mjs import fs from 'node:fs';
function getFs() {
return fs;
}
export { getFs }; my.test.mjs import assert from 'node:assert';
import { mock, test } from 'node:test';
// This works well
test('Mock shallow module', async () => {
for (let i = 0; i < 2; i++) {
const fsMock = mock.module('node:fs', { namedExports: { writeFileSync: mock.fn(() => i) } });
const fs = await import('node:fs');
assert.strictEqual(fs.writeFileSync(), i);
fsMock.restore();
}
});
// This fails
test('Mock deep module', async () => {
for (let i = 0; i < 2; i++) {
const fsMock = mock.module('node:fs', { namedExports: { writeFileSync: mock.fn(() => i) } });
const { getFs } = await import('./fs-extended.mjs');
const fs = getFs();
assert.strictEqual(fs.writeFileSync(), i);
fsMock.restore();
}
}); Result:
Even though I called I hope this is related to this issue, otherwise please let me know and I will file a new issue for my use-case. Edit: Tested on Node v22.9.0 using |
How do I invalidate the cache of
import
?I have a function that installs missing modules when an import fails, but the
import
statement seems to preserve the failure while the script is still running.The only information I found in regards to import caching was this documentation, which does not tell me where the “separate cache” used by
import
can be found.The text was updated successfully, but these errors were encountered: