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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

moduleNameMapper does not work with constants #8610

Closed
maksimsemenov opened this issue Jun 26, 2019 · 10 comments 路 Fixed by #9563
Closed

moduleNameMapper does not work with constants #8610

maksimsemenov opened this issue Jun 26, 2019 · 10 comments 路 Fixed by #9563

Comments

@maksimsemenov
Copy link

maksimsemenov commented Jun 26, 2019

馃悰 Bug Report

When I'm trying to use moduleNameMapper to resolve import from constants local folder, it does not work. It works fine, when I use singular form: constant.
I think it might happen because constants is sort of reserved module in node.js. However, plural form (constants) works fine with web pack aliases, so they find a workaround somehow.

To Reproduce

In one of the files import variable from constants module:

import { A_CONSTANT } from 'constants'

Set up an alias with moduleNameMapper:

"moduleNameMapper": {
      "^constants$": "<rootDir>/src/constants/index.js"
    }

Run tests

Expected behavior

The variable should be defined. Instead it is undefined

Link to repl or repo (highly encouraged)

https://github.com/maksimsemenov/jest-module-name-mapper

Run npx envinfo --preset jest

System:
    OS: macOS 10.14.5
    CPU: (4) x64 Intel(R) Core(TM) i5-4260U CPU @ 1.40GHz
  Binaries:
    Node: 10.15.3 - /opt/local/bin/node
    Yarn: 1.15.2 - /opt/local/bin/yarn
    npm: 6.9.0 - /usr/local/bin/npm
  npmPackages:
    jest: ^24.8.0 => 24.8.0 
@maksimsemenov maksimsemenov changed the title moduleNameMapper doe snot work with constants moduleNameMapper does not work with constants Jun 26, 2019
@nishantnaagnihotri
Copy link

I am facing this issue too.

The alias 'constants' does not work with Jest.
Any import from this alias comes as undefined during tests.

Jest: 23.6.0

@SimenB
Copy link
Member

SimenB commented Aug 13, 2019

Woah, this issue is super weird! The reason is that constants is a node core module

$ node -p "require('module').builtinModules.includes('constants')"
true

In that case, we give you that instead of the mapped module: https://github.com/facebook/jest/blob/94067089dd25c77463e24e981fe979fb6cc0b2fb/packages/jest-runtime/src/index.ts#L313-L315

I'm not sure how we should deal with this. We currently match node's behavior (even if you install e.g. fs from npm, doing require('fs') will silently give you the core module). Could we possibly throw an error if we resolve a local module, but figure out it's a node builtin?

/cc @thymikee @jeysal thoughts?

semi-related: nodejs/node#26012

@maksimsemenov
Copy link
Author

I think that the main confusion point, at least for me, was that I can normally use constants with webpack aliases, they resolve it to local modules instead of node builtin. Not sure which approach is more correct, but webpack's is more convenient: I can put all my constants in one folder and easily access them.

@jeysal
Copy link
Contributor

jeysal commented Aug 15, 2019

Intuitively, I'd assume moduleNameMapper overrides every aspect of the normal resolution like apparently it does in webpack, disregarding kinds of modules. And why not - if you want to use the respective core module in the same project, then ... umm, just don't configure the module name mapping like that 馃槄

@lh0x00
Copy link
Contributor

lh0x00 commented Feb 11, 2020

I suggest to add new field in the config with name is node and implement like webpack config but exclude mock value, only support: true, false and empty. This config will be used in _requireCoreModule function. If you think it is a good idea, I can help! :D

@jeysal
Copy link
Contributor

jeysal commented Feb 11, 2020

I don't think we need config for this, I think it should just be changed (fixed IMO). It should apply to module names like constants just like to any other module name.

@lh0x00
Copy link
Contributor

lh0x00 commented Feb 11, 2020

If the core modules are outside constants we will have many other names (os, util,...), this seems to be reasonable for this feature request! https://nodejs.org/api/index.html
I only think the case of a project that has been implemented and modified to use Jest fails as above. and I'm still the same as you, we shouldn't use those names in the first place!

@jeysal
Copy link
Contributor

jeysal commented Feb 11, 2020

I didn't quite understand your last message, I think you understood me, but just to confirm: IMO we should resolve this issue by changing from (pseudocode)

isCoreModule ? getCoreModule : (isMappedModule ? getMappedModule : getModule)

to

isMappedModule ? getMappedModule : (isCoreModule ? getCoreModule : getModule)

@lh0x00
Copy link
Contributor

lh0x00 commented Feb 11, 2020

Thanks, I will be create PR for it!

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants