atom/electron-link

__dirname doesn't work in modules that are part of the snapshot

astoilkov opened this issue · 6 comments

I noticed that __dirname returns relative paths instead of absolute when executed in a module that is part of the snapshot.

What is the reason of this happening? Is it possible this to be fixed or is it a limitation of the implementation?

Probably how the const dirname determines it's value is where this could be fixed.

function customRequire(modulePath) {
  let module = customRequire.cache[modulePath];
  if (!module) {
    module = { exports: {} };
    const dirname = modulePath.split('/').slice(0, -1).join('/');

    function define(callback) {
      callback(customRequire, module.exports, module);
    }

    if (customRequire.definitions.hasOwnProperty(modulePath)) {
      customRequire.cache[modulePath] = module;
      customRequire.definitions[modulePath].apply(module.exports, [module.exports, module, modulePath, dirname, customRequire, define]);
    } else {
      module.exports = require(modulePath);
      customRequire.cache[modulePath] = module;
    }
  }
  return module.exports;
}

I tried modifying the customRequire function with success. The __dirname started working properly​ again. Are you willing to accept a pull request for this?

@as-cii this seems reasonable to me but want your opinion.

I wasn't able to fix this problem because you need the __dirname when creating the snapshot not when requiring it. Maybe if __dirname get accessed from a function call which relies on a variable that comes from setGlobals() this may work.

Yes, at snapshot creation the __dirname path is a path relative to the main script. It is impossible to provide an absolute path during snapshot creation because snapshots are created on a computer, but could be executed on other computers, or the main executable could be moved to a different folder.

One workaround, as you're pointing out, could be to change the transformation routine to replace every instance of __dirname with get__dirname(relativePath) (as we do for forbidden modules). This new function will look something like:

function get__dirname(relativePath) {
   if (userHasSetAbsolutePath) {
      return path.join(absolutePath, relativePath)
   } else {
      return relativePath
   }
}

Then, at load time we could call setGlobals with the absolute path, so that any subsequent call to get__dirname (that is done at runtime) will return the correct absolute path.

Yes. This is what I was thinking. It would be awesome to fix this problem for future apps. In our case, we replaced all instances of __dirname with require.resolve() but this raises a problem that our developers in the team should be aware that they shouldn't use __dirname.

Thanks for the detailed suggestion and explanation @as-cii!