Issue with dynamically required files

[mdcarter]

I'm using a custom Hapi made server and I use this kind of require a lot (for loading plugins, methods and route :
await server.register(require(filepath))

I use this within loops that read folders and files to load everything dynamically.
It's not working (using run or build, either way), the required object/library is not available.

[guybedford]

There may be some things we can do here. It would help a lot of you could share how filepath is initially constructed and passed into the registration function in this example? Or is it entirely a user-API?

[mdcarter]

Of course, no problems.

Here is a sample code used to dynamically loads server methods and plugins to the Hapi server object :

await Promise.all([
  glob.sync(`./methods/**/index.js`).map(async filepath => {
    let method = require(filepath)
    await server.method(method.name, method.register)
  }),
  glob.sync(`./plugins/**/index.js`).map(async filepath => {
    await server.register(require(filepath))
  }),
])

[guybedford]

So this is a tricky case to apply static analysis to, as it requires glob-based knowledge for the static analysis. That is something that could be done eventually though, either directly, or through fs virtualization approaches that may be explored in future.

If the above were written to use something like require(path.resolve('./methods/' + relpath)) then that is something we can statically analyze right now.

[mdcarter]

@guybedford It's seems to be working fine the way you told me, thank you very much !
Now I juste have to wrap my head around exactly what's happening/if it's interesting for my project.
Thank you again !

[brettdh]

@guybedford I am having similar problems, but with third-party code:

https://spectrum.chat/zeit/now/error-with-require-a-particular-package-cannot-find-module-var-task-package-json~9fdea285-e149-4fdc-ad54-3e4eced7dc86

I may be able to apply a path.resolve workaround as you suggested (though I'd welcome a little more detail as to why that would work). Perhaps consider re-opening this to track future support for dynamic requires?

Perhaps also consider detecting such situations and warning/erroring? This took a while to track down, and an informative error message about this limitation would be really helpful.

[brettdh]

The relevant code: https://github.com/holidayextras/jsonapi-server/blob/master/lib/routes/index.js

[KevinWang15]

let's try eval("require")(filepath) ... 😓

[ghost]

@KevinWang15 Wow!!! How do you come up with it? 😀

[guybedford]

Note __non_webpack_require__ works in ncc since it is just webpack.

[KevinWang15]

seems like __non_webpack_require__ is compiled to exactly eval("require") by webpack 😀

[guybedford]

Yes, that's what we do in ncc internally :)

[Vadorequest]

FYI just read https://hackernoon.com/building-isomorphic-javascript-packages-1ba1c7e558c5 and adding yarn add -D @types/webpack-env fixed my TypeScript compilation failure when using __non_webpack_require__.

[songkeys]

@guybedford

So this is a tricky case to apply static analysis to, as it requires glob-based knowledge for the static analysis. That is something that could be done eventually though, either directly, or through fs virtualization approaches that may be explored in future.

Hi, this is a old thread but could you please be more specific about the vfs approaches? How would that work? I ran into a similar problem (a framework that loads js files dynamically according to the project structure) and tried to sovle it from ncc itself but failed.

(I've checked the vercel/webpack-asset-relocator-loader package but it seems that it recognises the js files as assets and relocating them without compiling them, which is not what I want. I have no clue about what to do now.)

Thank you.

[guybedford]

@songkeys each case is very specific here, so if you have a code example of why eggjs is not properly bundling that would help to provide suggestions further.

[songkeys]

@guybedford
I think the issue is still about dynamic requiring. This PR seems to have fixed it? But actually, I tested the following script and it's not.

require('xxx') // this will be recognised

const dynamicRequire = (packageName) => require(packageName)
dynamicRequire('xxx') // this won't be recognised

I'd like to know what did you mean by "fs virtualization approaches". Because in my case the framework works like this to require a relative path:

const requireFile = (fileName) => {
  /* glob to parse fileName */
  require(fileName)
}
requireFile('./controller/*.js')

"fs virtualization approaches" sounds like a way to solve this as I can make the final output file think it had a fold structure and each file was in the original places?

---

update:

seems that the cases are complex for ncc to understand, even the test file failed:

I take the test file to test, here is the output:

{

/***/ 347:
/***/ (function(__unusedmodule, __unusedexports, __webpack_require__) {

const reaction = (name) => {
    const res = require(name);
    res.name = name.split("/").pop();
    return res;
},reaction$$mod = (nres, name) => { // note the first argument is `nres` but not the expected `res`, which fails the file!!
    
    res.name = name.split("/").pop();
    return res;
};

const reactions = {
    repository: {
        publicized: reaction$$mod(__webpack_require__(711), "./dep"),
    },
};


/***/ }),

/***/ 711:
/***/ (function(module) {

module.exports = "dep";


/***/ })

/******/ }

And here are some other test I've done

// case 1: same case as above but simplified
const dynamicRequire = (path) => {
    const res = require(path);
    return res;
};

dynamicRequire("./dep"); // seems working but not (result as above)

// case 2: same case as above but without `const` the function
dynamicRequire = (path) => {
    const res = require(path);
    return res;
};

dynamicRequire("./dep"); // not works!

// case 3: same case as above but directly returning the required result
const dynamicRequire = (name) => {
    return require(name);
};

dynamicRequire("./dep"); // not works!

// case 4: same case as above but directly assigning the require function
dynamicRequire = require;
dynamicRequire("./dep"); // not works!

I raised a new issue: #578

[KevinWang15]

PoC of file system virtualization

bundler

const fs = require("fs");
const path = require("path");
const {GlobSync} = require("glob");
process.chdir("./target");


const requireRedefined = dirName => `function require(file) {
    const module={};
    return eval(fileMap['./' + path.relative(process.cwd(), path.resolve('${dirName}/'+file))] + 'module.exports;');
}
`;

const fileMap = {};
const files = GlobSync("./**/*.js").found;
for (let file of files) {
    fileMap[file] = requireRedefined(path.dirname(file)) + fs.readFileSync(file, {encoding: "UTF-8"});
}

const resultFile = [
    `const path = require('path');`,
    `const fileMap = ${JSON.stringify(fileMap, null, 4)};`,
    `eval(fileMap["./index.js"]);`
].join("\n");

process.stdout.write(resultFile);

input

./index.js
["a.js", "b.js"].forEach(file => {
    console.log(require("./dependencies/" + file));
});

./dependencies/a.js
const a1 = require("./dependencies/a1.js")
module.exports = "AAA" + a1;

./dependencies/dependencies/a1.js
module.exports = "A1";

./dependencies/b.js
module.exports = "BBB";

output

const path = require('path');
const fileMap = {
    "./dependencies/a.js": "function require(file) {\n    const module={};\n    return eval(fileMap['./' + path.relative(process.cwd(), path.resolve('./dependencies/'+file))] + 'module.exports;');\n}\nconst a1 = require(\"./dependencies/a1.js\")\nmodule.exports = \"AAA\" + a1;",
    "./dependencies/b.js": "function require(file) {\n    const module={};\n    return eval(fileMap['./' + path.relative(process.cwd(), path.resolve('./dependencies/'+file))] + 'module.exports;');\n}\nmodule.exports = \"BBB\";",
    "./dependencies/dependencies/a1.js": "function require(file) {\n    const module={};\n    return eval(fileMap['./' + path.relative(process.cwd(), path.resolve('./dependencies/dependencies/'+file))] + 'module.exports;');\n}\nmodule.exports = \"A1\";",
    "./index.js": "function require(file) {\n    const module={};\n    return eval(fileMap['./' + path.relative(process.cwd(), path.resolve('./'+file))] + 'module.exports;');\n}\n[\"a.js\", \"b.js\"].forEach(file => {\n    console.log(require(\"./dependencies/\" + file));\n});\n"
};
eval(fileMap["./index.js"]);

an interesting concept worth exploring.. @songkeys

[guybedford]

Thanks for the clear reports. The problem in the eggjs case is dynamic require detection over virtualization it looks like to me. I believe some of these dynamic require cases are handled better by the Webpack upgrade in #579 since Webpack improves the dynamic require detection handling in one of those releases. I'd suggest trying that out once it lands then reposting a new issue if it still isn't working.

[songkeys]

@guybedford Thank you. I've tried the latest version (0.24.0) from #579. It handles dynamic requiring better but not perfectly. Anyway, think the issue #578 can be closed now.

The framework I'm using checks the resource folders/files on runtime. For example, the project structure is like:

- app/
   - controller/
   - router.js
   - index.js

There is no "direct" require statement to point out the files (e.g. controller directory or router.js) and import them into the framework. The framework checks if there exists such a file/folder on runtime (e.g. path.resolve(process.cwd(), 'controller')) and then requires them. Since the project structure is destroyed by ncc into a single file, it never gonna find the expected files.

I don't think this case could be handled by static analyzation. The PoC of file system virtualization proposed by @KevinWang15 is more like a solution to me.

Let me know if you need more detail or a new issue.

[devthejo]

here is my PoC of a 2 pass working solution:

each dynamic require have to run one time before ncc build
won't work with conditional runtime requires, but useful for files structure based tools

index.js

const path = require("path")
const fs = require("fs")

const dynamicFilename = `${process.cwd()}/build/requires.js`
const dynamicRequires = new Set()
const dynamicRequireRegister = (src)=>{
  if (dynamicRequires.has(src)){
    return
  }
  dynamicRequires.add(src)
  const exports = [...dynamicRequires].reduce((acc, req)=>{
    const target = !req.startsWith(".") ? req : path.join(process.cwd(), req)
    
    acc.push(`${JSON.stringify(req)}: require(${JSON.stringify(target)})`)
    return acc
  },[])
  fs.mkdirSync(path.dirname(dynamicFilename),{recursive:true})
  fs.writeFileSync(dynamicFilename, `module.exports={${exports.join(",")}}`)
  delete require.cache[require.resolve(dynamicFilename)]
}
const dynamicRequire = (r)=>{
  let required
  try {
    // build time, each dynamic require have to run one time before ncc build
    // won't work with conditional runtime requires, but useful for files structure based tools
    require.resolve(r)
    required = true
  }
  catch(_e){
    // do nothing
  }
  if (required){
    dynamicRequireRegister(r)
  }
  // the require target has to be static, dont replace it with variable dynamicFilename
  return require(`${process.cwd()}/build/requires.js`)[r]
}

const r = "./my-dynamic-require.js"

const hello = dynamicRequire(r)

if(process.env.BUILD){
  return
}

hello("folks")

my-dynamic-require.js

module.exports = (some) => console.log(`Hello ${some} !`)

then call

BUILD=1 node index.js && ncc build index.js

and you're good ;)

node dist/index.js

[songkeys]

@devthejo i think we share the same thought. #734 guybedford has given his suggestion on this idea.