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

Closed
wants to merge 6 commits into from
Closed

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
Loading

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
Loading

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
Loading

@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
Loading

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

@goodmind goodmind force-pushed the flowtyped-js.flow branch from d4176e0 to 1ac6c8b Compare June 21, 2019 20:18
@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
@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 04467ee.

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 Merged module resolution
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@goodmind @jedwards1211 @facebook-github-bot @nmote