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.

Sign up for GitHub

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

Jump to bottom

Add @flowtyped scope resolution before actual Node resolution #7758

Open
wants to merge 6 commits into
base: main
Choose a base branch
from
Open

Add @flowtyped scope resolution before actual Node resolution #7758

wants to merge 6 commits into from

Conversation

goodmind
Copy link
Contributor

@goodmind goodmind commented May 24, 2019 •
edited

Proof of concept for #7750

Issues

  • Depends on flow-typed NPM migration

  • Clashes with (in particular if there would be migration period from old libdefs to .js.flow)

[libs]
node_modules/@flowtyped

@nmote nmote self-assigned this May 24, 2019
@goodmind
Copy link
Contributor Author

@nmote this doesn't actually work if this and [libs] .json fix both get merged, because @flowtyped is still using libdefs

@goodmind
Copy link
Contributor Author

goodmind commented Jun 2, 2019 •
edited

I tried to implement there this feature request #4917. Not sure how slower this makes dependency resolving, this basically reads package.json on every import

@goodmind goodmind changed the title Add @flowtyped scope resolution before actual Node resolution Add @flowtyped scope resolution before actual Node resolution, implement versioned .js.flow Jun 2, 2019
@goodmind
Copy link
Contributor Author

goodmind commented Jun 2, 2019

@jedwards1211 I think you would be interested to check this out?

@jedwards1211
Copy link
Contributor

Thanks, though I've never found time to learn much OCaml so it's hard for me to tell exactly what this is doing...what do you mean by versioned .js.flow? I didn't see anything in your proposal repo about it

@goodmind
Copy link
Contributor Author

goodmind commented Jun 4, 2019 •
edited

@jedwards1211 check tests in this PR, they're in JS. I still didn't updated proposal, because when I profiled this it increases time of dependency resolution significantly :\

@goodmind
Copy link
Contributor Author

goodmind commented Jun 4, 2019 •
edited

https://github.com/facebook/flow/tree/d4176e0a9b0487bd1abee050d0db2fba94521bfd/tests/declaration_files_node_versions
Particurarly package.json of node_modules

@goodmind goodmind changed the title Add @flowtyped scope resolution before actual Node resolution, implement versioned .js.flow Add @flowtyped scope resolution before actual Node resolution Jun 21, 2019
@nmote nmote removed their assignment Mar 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
CLA Signed module resolution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@goodmind @jedwards1211 @facebook-github-bot @nmote