-
Notifications
You must be signed in to change notification settings - Fork 1.9k
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
Setting content root causes modules to be not found #8156
Comments
Thanks a ton for taking the time to put this together! I took a look at your repro, and I'm seeing the same behavior I noticed before If I git clone the repro and run
If I run
In the IDE, when I open the files I see the same errors AFAICT, If you set Setting However, putting your source files inside of a directory which Flow treats as a So my thoughts are that two things are appropriate here:
|
@gabelevi Also setting the |
Another issue with this is that Flow can't resolve any node modules even with empty flowconfig. |
That's actually a little reassuring. I was having trouble understanding how live non-parse errors (e.g. live type errors) was interacting with this issue. |
And yeah, that name mapper solution would turn any import that doesn't start with a |
Summary: #8156 made me aware that some users want to be able to import their code using root-relative paths. They were misusing `module.system.node.resolve_dirname`, which worked for them due to a bug, and experiencing odd behaviors. This stack of diffs demonstrates the behavior, fixes the bug (breaking the use case), and adds first-class support for this use case. This diff adds a test demonstrating the current behavior. Reviewed By: jbrown215 Differential Revision: D18171800 fbshipit-source-id: a43e8286c78fd2196980c211b38a347450575789
Summary: #8156 made me aware that some users want to be able to import their code using root-relative paths. They were misusing `module.system.node.resolve_dirname`, which worked for them due to a bug, and experiencing odd behaviors. This stack of diffs demonstrates the behavior, fixes the bug (breaking the use case), and adds first-class support for this use case. This diff fixes the bug. We kept track of which directories contain `node_modules` (or one of its aliases), but we didn't keep track of which alias is within the container. This meant that if you configured your `.flowconfig` with ``` module.system.node.resolve_dirname=node_modules module.system.node.resolve_dirname=. ``` then any directory `path/to/container` that contained `node_modules/` would lead to us thinking `path/to/container/./` contained node modules. This allowed people to use root-relative paths, but also treated all the code as if it were inside `node_modules/` The fix is to keep track of which aliases we saw within each container. Reviewed By: jbrown215 Differential Revision: D18172874 fbshipit-source-id: e6851c16b811d1160f65b647d2bb123649c7e5fd
Summary: #8156 made me aware that some users want to be able to import their code using root-relative paths. They were misusing `module.system.node.resolve_dirname`, which worked for them due to a bug, and experiencing odd behaviors. This stack of diffs demonstrates the behavior, fixes the bug (breaking the use case), and adds first-class support for this use case. This diff adds a new `.flowconfig` option which allows you to import modules using a root-relative path. Reviewed By: jbrown215 Differential Revision: D18190689 fbshipit-source-id: b1a81e39faa414e3208ae4c804cc3f9aeb91cbcf
Summary: #8156 made me aware that some users want to be able to import their code using root-relative paths. They were misusing `module.system.node.resolve_dirname`, which worked for them due to a bug, and experiencing odd behaviors. This stack of diffs demonstrates the behavior, fixes the bug (breaking the use case), and adds first-class support for this use case. This diff makes `module.system.node.resolve_dirname=.` error. You're only supposed to be passing in valid directory names anyway. `.` and `..` are reserved. Reviewed By: jbrown215 Differential Revision: D18191349 fbshipit-source-id: 72e3efde4d2f832974bd34e2d6a793cebb5fefbb
Summary: #8156 made me aware that some users want to be able to import their code using root-relative paths. They were misusing `module.system.node.resolve_dirname`, which worked for them due to a bug, and experiencing odd behaviors. This stack of diffs demonstrates the behavior, fixes the bug (breaking the use case), and adds first-class support for this use case. This diff adds a test demonstrating the current behavior. Reviewed By: jbrown215 Differential Revision: D18171800 fbshipit-source-id: d243b0d429798e3c5b57a21834a7e203d71860a1
Summary: #8156 made me aware that some users want to be able to import their code using root-relative paths. They were misusing `module.system.node.resolve_dirname`, which worked for them due to a bug, and experiencing odd behaviors. This stack of diffs demonstrates the behavior, fixes the bug (breaking the use case), and adds first-class support for this use case. This diff fixes the bug. We kept track of which directories contain `node_modules` (or one of its aliases), but we didn't keep track of which alias is within the container. This meant that if you configured your `.flowconfig` with ``` module.system.node.resolve_dirname=node_modules module.system.node.resolve_dirname=. ``` then any directory `path/to/container` that contained `node_modules/` would lead to us thinking `path/to/container/./` contained node modules. This allowed people to use root-relative paths, but also treated all the code as if it were inside `node_modules/` The fix is to keep track of which aliases we saw within each container. Reviewed By: jbrown215 Differential Revision: D18172874 fbshipit-source-id: c76b8e8b653312d56df078fb57ace9ba56bddd57
Summary: #8156 made me aware that some users want to be able to import their code using root-relative paths. They were misusing `module.system.node.resolve_dirname`, which worked for them due to a bug, and experiencing odd behaviors. This stack of diffs demonstrates the behavior, fixes the bug (breaking the use case), and adds first-class support for this use case. This diff adds a new `.flowconfig` option which allows you to import modules using a root-relative path. Reviewed By: jbrown215 Differential Revision: D18190689 fbshipit-source-id: e61325ee847361a2873327b8302f3662799d6f58
Summary: #8156 made me aware that some users want to be able to import their code using root-relative paths. They were misusing `module.system.node.resolve_dirname`, which worked for them due to a bug, and experiencing odd behaviors. This stack of diffs demonstrates the behavior, fixes the bug (breaking the use case), and adds first-class support for this use case. This diff makes `module.system.node.resolve_dirname=.` error. You're only supposed to be passing in valid directory names anyway. `.` and `..` are reserved. Reviewed By: jbrown215 Differential Revision: D18191349 fbshipit-source-id: 716bfd9705bd691332cae0d5ef75ba69601e822b
Ok! This should be done now! |
Nice, thanks a lot @gabelevi! |
I imagine this will trip up a bunch of people. What we did to fix it:
|
Flow version: 0.110.0 ->
Expected behavior
Flow should understand path patterns like previous versions does.
Actual behavior
Flow says
It's quite common pattern to reduce complexity of imports by setting project root as absolute root so that it can be used in imports. In 0.110.0 -> versions Flow fails these cases.
You can repro the issue by cloning the repo, and opening
js2/test.js
file in editor. Interestingly,yarn flow
does not give any errors, which is expected behaviour.This is probably the original message that sparked such settings: #4186 (comment) and this article has made it more popular: https://medium.com/@sherryhsu/how-to-change-relative-paths-to-absolute-paths-for-imports-32ba6cce18a5
E: create-react-app suggest following: https://create-react-app.dev/docs/adding-flow/
module.name_mapper='^\([^\.].*\)$' -> '<PROJECT_ROOT>/src/\1'
The text was updated successfully, but these errors were encountered: