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

[Regression from 0.45 -> 0.46] resolve_dirname no longer applies #4103

Closed
DanielHeath opened this issue Jun 6, 2017 · 10 comments
Closed

[Regression from 0.45 -> 0.46] resolve_dirname no longer applies #4103

DanielHeath opened this issue Jun 6, 2017 · 10 comments

Comments

@DanielHeath
Copy link

After upgrading from 0.45 to 0.46, we have hundreds of Required module not found errors.

The relevant flowconfig snippet is:

[options]
module.system.node.resolve_dirname=src/js
module.system.node.resolve_dirname=test/unit

An example of the new error:

src/js/client/<snip>/<snip>/<snip>/__tests__/index-spec.js:3
  3: import { within } from 'helpers/stubs';
                            ^^^^^^^^^^^^^^^ helpers/stubs. Required module not found

This file is present:

  $ ls test/unit/helpers/stubs.js
test/unit/helpers/stubs.js
@gabelevi
Copy link
Contributor

gabelevi commented Jun 6, 2017

Can confirm this repros with https://gist.github.com/gabelevi/edc83db958db739fdf81749f205877b5 and that the issue was introduced with v0.46.0

@avikchaudhuri - was this your moving of require resolution that you landed near the end of April?

@valscion
Copy link
Contributor

We are facing the same regression and as a result almost every one of our files have errors.

Is there anything I could do to help? I could whip up a reproduction repository if that helps.

An alternative we are considering is to stop using resolve_dirname to completely hide the root folder and introduce a new variable, say appJS, that would point to our JS root that we could use.

# .flowconfig

[options]
module.name_mapper='appJS' -> '<PROJECT_ROOT>/app/assets/javascripts'
// webpack
return {
  resolve: {
    alias: {
      appJS: path.join(projectRoot, 'app/assets/javascripts')
    }
  }
};

@leethree
Copy link

Got the same problem. This no longer works after upgrade beyond 0.45.

module.system.node.resolve_dirname=.

@valscion
Copy link
Contributor

The way we resolved it for now was to list out all the "root" directories separately with module.name_mapper option.

So instead of

module.system.node.resolve_dirname=app/assets/javascripts

we have this:

module.name_mapper='boot' -> '<PROJECT_ROOT>/app/assets/javascripts/boot'
module.name_mapper='common' -> '<PROJECT_ROOT>/app/assets/javascripts/common'
module.name_mapper='conversation' -> '<PROJECT_ROOT>/app/assets/javascripts/conversation'
# ... and so on ...

It isn't nearly as nice as having the resolve_dirname work as before, as now when we want to add a new root directory, we will need to amend this list of name mappers to cover it.

@valscion
Copy link
Contributor

Looks like the functionality this regression issue points out has been added back, with a combination like this:

module.system.node.allow_root_relative=true
module.system.node.root_relative_dirname=./app

(Example copied from #8156 (comment))

We were able to get rid of our

module.name_mapper='boot' -> '<PROJECT_ROOT>/js/src/boot'
module.name_mapper='common' -> '<PROJECT_ROOT>/js/src/common'
module.name_mapper='conversation' -> '<PROJECT_ROOT>/js/src/conversation'
....

configuration by changing the .flowconfig options to this:

module.system.node.allow_root_relative=true
module.system.node.root_relative_dirname=./js/src

References:

Pull requests that never got merged but tried to get this feature back:

Sooooo... is this issue fixed now? Granted, it was a bit difficult to find out the correct configuration options to use as the docs haven't been updated yet.

@spoulson
Copy link

Can confirm resolve_dirname broken as of 0.111.0. My .flowconfig:

[options]
module.system.node.resolve_dirname=node_modules
module.system.node.resolve_dirname=src/libs

Getting "Cannot resolve module" errors on 0.111.0. No error on prior versions.

@valscion
Copy link
Contributor

Did you try the other approach I defined above, @spoulson?

@spoulson
Copy link

@valscion Yes, the name_mapper setting does work. But, it's not ideal because I need to 1) update all my repos that use previously working resolve_dirname and 2) explicitly state the individual module mappings, which is a configuration burden. I enjoyed the ability to just point to a relative directory and add that to the module import paths.

Technically, I can migrate to name_mapper, but if resolve_dirname is supported functionality I'd rather use that.

@spoulson
Copy link

@TrySound Why close this bug?

@TrySound
Copy link
Contributor

resolve_dirname is not supposed to be used this way. It allows to specify node_modules alias. For node_modules flow does not watch file changes. This is implemented to solve your problem

module.system.node.allow_root_relative=true
module.system.node.root_relative_dirname=./js/src

So it's not really a bug. The change was made intentionally to solve performance issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants