nodejs / node

Node.js JavaScript runtime βœ¨πŸ’πŸš€βœ¨
https://nodejs.org
Other
106.57k stars 29.05k forks source link

import(cjs) and require(cjs) share a cache #49453

Open SimenB opened 4 years ago

SimenB commented 4 years ago

Hiya! πŸ‘‹

This might very well be on purpose, and if so feel free to close this issue.

For background, I'm currently working on ESM support in Jest. We implement our own resolution and module loading (including caching), so I'm writing some tests to try to make sure our semantics matches node's semantics.

I wrote the following test:

// stateful.cjs
'use strict';

let num = 0;

module.exports = function inc() {
  num++;
  return num;
};
// test.mjs
import * as assert from 'assert';
import {createRequire} from 'module';

async function main() {
  const require = createRequire(import.meta.url);

  const requiredStateful = require('./stateful.cjs');
  // delete require.cache[require.resolve('./stateful.cjs')];
  const {default: importedStateful} = await import('./stateful.cjs');

  assert.equal(importedStateful(), 1);
  assert.equal(importedStateful(), 2);
  assert.equal(requiredStateful(), 1);
  assert.equal(importedStateful(), 3);
  assert.equal(requiredStateful(), 2);
  assert.equal(requiredStateful(), 3);
}

main().catch(err => {
  process.exitCode = 1;

  console.error(err);
});

However, this test fails because the numbers are 1, 2, 3, 4, 5, 6, e.g. they share the same module rather than having their own.

I assumed that the module cache of require and import() would not be shared, based on

require.cache is not used by import. It has a separate cache.

From https://nodejs.org/api/esm.html#esm_no_require_cache

Note that if I swap the order from require then import to import then require (and uncomment the delete), the test behaves like I expect.

So doing import(cjs) populates require.cache which means require(cjs) loads the cached module, and require(cjs) populates the cache so import(cjs) reads it, but doing delete require.cache only affect require, not import.

If import populates and reads from require.cache, I'd expect it do respect deletions from that same property. What I would expect from reading the docs is that import would neither populate nor read from require.cache.

I ran these tests with no flags, using node v13.12.0.


Again, this might be expected behavior, and if so I'd be happy to provide a PR with a docs change specifying this. As a user I find it quite confusing, though

jkrems commented 4 years ago

require.cache is not used by import. It has a separate cache.

Yeah, that quote is technically correct which is the worst kind of correct.

Part of the puzzle is that the import cache is separate but never contains CommonJS (!). An attempt of pseudo code:

function import(specifier, baseURL) {
  const resourceURL = resolve(specifier, baseURL);

  // This is a separate cache from `require.cache`.
  importCache[resourceURL] ??= (() => {
    let {type, content} = loadResource(resourceURL);
    if (type === 'commonjs') {
      content = `\
import {createRequire} from 'module';
import {fileURLToPath} from 'url';
const require = createRequire(import.meta.url);
// This is using require, including require.cache.
export default require(fileURLToPath(import.meta.url));
`;
    }
    return createModuleFromSource(type, content);
  })();
  return importCache[resourceURL];
}

So the import cache is separate and isn't affected by require.cache but it doesn't contain the CommonJS module. It contains a wrapper module that returns the result of require-ing the same resource at the time of module execution. And the require call in that wrapper module uses and is affected by require.cache.

I hope that shed some light and didn't add to the confusion. The docs could definitely use some clarification here.

jkrems commented 4 years ago

Some clarification: The pseudo code is for the general idea. The current implementation takes short-cuts instead of literally generating that source text (it uses Module::CreateSyntheticModule). But afaict the observable behavior is pretty much the above. The point is that there's nothing in the ES module graph that really runs the CommonJS code directly, it's deferring to the CJS loader for that.

SimenB commented 4 years ago

Not sure I still really follow. Why does import read and write to some internal cjs cache? I understand it internally uses the same mechanisms to load, but why doesn't it use its own cache exclusively?

MylesBorins commented 4 years ago

Part of the cache is shared, through reference, so that there is only a single instance of a cjs Singleton.

If you require then import the same cjs module, you get the same Singleton

On Sun, Apr 19, 2020, 2:06 PM Simen Bekkhus notifications@github.com wrote:

Not sure I still really follow. Why does import read and write to some internal cjs cache? I understand it internally uses the same mechanisms to load, but why doesn't it use its own cache exclusively

β€” You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/nodejs/node/issues/49453, or unsubscribe https://github.com/notifications/unsubscribe-auth/AADZYVZBVZ2J6DOXGRXQ3F3RNM4RTANCNFSM4MLWL3XQ .

SimenB commented 4 years ago

Jest doesn't support require.cache anyways (we have explicit cache clearing APIs), so if I can treat import(cjs) and require(cjs) the same it actually simplifies the implementation.

I think this should be specified in the docs, but I'm not entirely sure about how. I can give it a whack tomorrow unless somebody else feels up for it πŸ‘