Skip to content

Odd behaviour: createRequireFromPath('.') #27401

Closed
Closed
Odd behaviour: createRequireFromPath('.')#27401
Labels
feature requestIssues requesting new Node.js features.moduleIssues and PRs related to the module subsystem.

Description

@SMotaal
SMotaal
opened on Apr 24, 2019
  • Version: v12.0.0
  • Platform: macOS 10.14.4
  • Subsystem: module

Calling createRequireFromPath('.') does not honour process.cwd() when called a second time after an effective call to process.chdir(…).

Best explained by this: https://github.com/SMotaal/esm/blob/master/tests/create-require.mjs#L8

Details 
import {createRequireFromPath} from 'module';
import {fileURLToPath} from 'url';

const specifier = './package.json';
const [scope, ...paths] = ['../../', '../', './'].map(path => fileURLToPath(new URL(path, import.meta.url)));
const sanitize = path => path.replace(scope, '‹scope›/');

for (const path of process.argv.includes('-t2') ? paths.reverse() : paths) {
  process.chdir(path);
  console.group('\nprocess.chdir(%o)', sanitize(process.cwd()));
  console.log();
  try {
    const require = createRequireFromPath('.');
    console.log(
      `createRequireFromPath(%o)\n  .resolve(%o)\n    => %o`,
      '.',
      specifier,
      sanitize(require.resolve(specifier)),
    );
  } catch (exception) {
    console.warn(
      `createRequireFromPath(%o)\n  .resolve(%o)\n    => %o`,
      '.',
      specifier,
      `${exception}`.split('\n', 1)[0],
    );
  }
  console.log();
  console.groupEnd();
}

/*******************************************************************************
 * $ node --experimental-modules esm/tests/create-require.mjs
 *
 *   process.chdir('‹scope›/esm/')
 *
 *     createRequireFromPath('.')
 *       .resolve('./package.json')
 *         => '‹scope›/esm/package.json'
 *
 *   process.chdir('‹scope›/esm/tests/')
 *
 *     createRequireFromPath('.')
 *       .resolve('./package.json')
 *         => '‹scope›/esm/package.json'
 *
 ******************************************************************************
 * $ node --experimental-modules esm/tests/create-require.mjs -t2
 *
 *   process.chdir('‹scope›/esm/tests/')
 *
 *     createRequireFromPath('.')
 *       .resolve('./package.json')
 *         => '‹scope›/esm/tests/package.json'
 *
 *   process.chdir('‹scope›/esm/')
 *
 *     createRequireFromPath('.')
 *       .resolve('./package.json')
 *         => '‹scope›/esm/tests/package.json'
 *
 ******************************************************************************/

Activity

  1. BridgeAR
    added
    feature requestIssues requesting new Node.js features.
    moduleIssues and PRs related to the module subsystem.
    on Apr 24, 2019

  2. BridgeAR commented on Apr 24, 2019

    @BridgeAR
    BridgeAR
    on Apr 24, 2019 · Hidden as outdated

  3. BridgeAR commented on Apr 25, 2019

    @BridgeAR
    BridgeAR
    on Apr 25, 2019
    Member

    This is actually working as intended after looking deeper into this. To path provided is the relative path to the current file or an absolute path. AFAIK this should not honor changing the working directory as it's independent.

  4. MylesBorins commented on Apr 25, 2019

    @MylesBorins
    MylesBorins
    on Apr 25, 2019
    Contributor

    I think there is a slightly more subtle bug behavior here... in that createRequireFromPath('.') seems to always use process.cwd() as the base 😅

    indirection/lol.js

    const {createRequireFromPath} = require('module');
const req = createRequireFromPath('.');

console.log(process.cwd());
console.log(require.resolve('./package.json'));
console.log(req.resolve('./package.json'));
    $ node indirection/lol.js
/Users/mylesborins/code/makeRequireFromPath
/Users/mylesborins/code/makeRequireFromPath/indirection/package.json
/Users/mylesborins/code/makeRequireFromPath/package.json
$ cd indirection
$ node lol.js
/Users/mylesborins/code/makeRequireFromPath/indirection
/Users/mylesborins/code/makeRequireFromPath/indirection/package.json
/Users/mylesborins/code/makeRequireFromPath/indirection/package.json

    edit: this is because createRequireFromPath(path) calls new Module(path) which is turn calls path.dirname(path) which when called is relative to the CWD, not the filename of the callee

  5. devsnek commented on Apr 25, 2019

    @devsnek
    devsnek
    on Apr 25, 2019
    Member

    I don't think there should be any "callee" magic here. my intention when I originally proposed this feature was something along the lines of makeRequire(import.meta.url).

  6. MylesBorins commented on Apr 25, 2019

    @MylesBorins
    MylesBorins
    on Apr 25, 2019
    Contributor

    @devsnek but as the feature is documented it doesn't really work for two reasons

    from docs

    const { createRequireFromPath } = require('module');
const requireUtil = createRequireFromPath('../src/utils');

// Require `../src/utils/some-tool`
requireUtil('./some-tool');

    issue 1) if you create a require from a relative path it is going to be different depending on cwd(), likely unexpected

    issue 2) directories don't actually work like that

    Seems like we need to revisit some of the behavior here imho

  7. SMotaal commented on Apr 25, 2019

    @SMotaal
    SMotaal
    on Apr 25, 2019
    Author

    @BridgeAR I just wanted to clarify the behaviour:

    1. running node … esm/tests/create-require.mjs from '‹scope›'/

      • it calls process.chdir(`‹scope›/esm/`) first
      • it calls createRequireFromPath('.').resolve('./package.json')
      • this returns '‹scope›/esm/package.json'
      • it then calls process.chdir(`‹scope›/esm/test/`) second
      • it calls createRequireFromPath('.').resolve('./package.json')
      • this returns '‹scope›/esm/package.json' also

    2. running node … esm/tests/create-require.mjs -t2 from '‹scope›'/

      • it calls process.chdir(`‹scope›/esm/test/`) first
      • it calls createRequireFromPath('.').resolve('./package.json')
      • this returns '‹scope›/esm/test/package.json'
      • it then calls process.chdir(`‹scope›/esm/`) second
      • it calls createRequireFromPath('.').resolve('./package.json')
      • this returns '‹scope›/esm/test/package.json' also

    There is clearly a cached aspect which:

    1. does not get initialized until first call to createRequireFromPath
    2. picks up process.cwd() at that time (at least with '.' as the arg).
    3. since 2 happens, then process.chdir(…) not affecting next call to a new createRequireFromPath - irrespective of why the user wanted 2 of them — is not a behaviour that is clear from a user perspective.
    4. if 2 is not intended, then '.' not resolving to the current module — irrespective of this being CJS/ESM — is not a behaviour that is helpful from a user perspective.

    So I was really hoping we can clarify the behaviour of '.' here.

    Hope this was helpful 😄

    Side note: The few times, I wanted to chdir in node, I always need to remind myself that process.chdir(…) not process.cwd(…) — ie I maybe do this once every 1-2 years and usually to test odd behaviour of resolved paths seemingly tied to process.cwd().

    So, I would be favourable of module related utilities being more aware of module locations than process.cwd is my intuitive feel for moving forward here.

  8. SMotaal commented on Apr 25, 2019

    @SMotaal
    SMotaal
    on Apr 25, 2019
    Author

    The cached behaviour in your opinion, does that qualify as a bug?

  9. MylesBorins commented on Jun 12, 2019

    @MylesBorins
    MylesBorins
    on Jun 12, 2019
    Contributor

    Closing as I believe this has been fixed

  10. MylesBorins
    closed this as completedon Jun 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    feature requestIssues requesting new Node.js features.moduleIssues and PRs related to the module subsystem.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions