Skip to content
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

require(esm) always feeds full filepaths to customization hooks but require.resolve doesn't #57125

Open
bmeck opened this issue Feb 18, 2025 · 2 comments
Labels
loaders Issues and PRs related to ES module loaders

Comments

@bmeck
Copy link
Member

bmeck commented Feb 18, 2025

Version

v23.7.0

Platform

Darwin X-MacBook-Pro.local 23.1.0 Darwin Kernel Version 23.1.0: Mon Oct  9 21:27:24 PDT 2023; root:xnu-10002.41.9~6/RELEASE_ARM64_T6000 arm64

Subsystem

module

What steps will reproduce the bug?

Create a customization hook that intercepts resolve and log out the arguments while making some CJS with the patched require from being loaded via ESM; notice that randomly even though it is not possible to resolve to file: you get a file specifier (added some CJS hooks to show better explanation [note _load called differently in non-requirable ESM CJS]):

const Module = require('module');

// Save the original loader so we can call it later.
const originalLoad = Module._load;
const originalResolveFilename = Module._resolveFilename;

Module._resolveFilename = function(request, parent) {
  console.log({_resolveFilename:{request,parent}})
  return 'foo'
  return originalResolveFilename.apply(this, arguments);
};
Module._load = function(request, parent, isMain) {
console.log({_load:{request,parent,isMain}})
  const exported = originalLoad.apply(this, arguments);
  return exported;
};

let id = 1
Module.registerHooks({
    resolve(specifier, context, nextResolve) {
        console.dir({resolve:{specifier,context}}, {depth:null})
        if (specifier.startsWith('file:')) {
            debugger;
        }
        return {
            url: `sea:${id++}`,
            shortCircuit: true
        }
        return nextResolve(specifier, context)
    },
    load(url, context, nextLoad) {
        console.dir({load:{url,context}}, {depth:null})
        return {
            source: `
                if (module.id.endsWith(2)) throw Error('bail');
                let dep = "sea:bare/${String(url).slice(4)}"
                try {
                    require(dep);
                } catch (error) {
                 // NOTE: this gives a different path to load!
                    console.log('FAILED TO REQUIRE', dep, require.resolve(dep))
                }
            `,
            format: 'commonjs',
            shortCircuit: true
        }
    }
})
import('bare')

How often does it reproduce? Is there a required condition?

everytime

What is the expected behavior? Why is that the expected behavior?

CJS (either by default or hooked _resolveFilename resolves to a non-file [e.g. foo]) and then that value is passed into the loader hooks

What do you see instead?

  1. IDEAL: don't need to hook into _resolveFilename to make resolution work; (removing the above monkey patch to _resolveFilename still causes failure)
  2. unify logic of so that instrumentation can properly deduce how to respond
    i. due to personal preference I'd suggest if they differ to respect the altered form rather than trying to change it into a file.
    ii. There is some odd logic around WASM and JSON that doesn't match either.

Additional information

No response

@joyeecheung
Copy link
Member

I am guessing this has something to do with the resolve hook being run again in the evaluation callback of import(cjs). Recognizing this case and skip the second resolution hook might make it go away.

@joyeecheung joyeecheung added the loaders Issues and PRs related to ES module loaders label Feb 18, 2025
@joyeecheung
Copy link
Member

Likely intertwined with #55808

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
loaders Issues and PRs related to ES module loaders
Projects
None yet
Development

No branches or pull requests

2 participants