Skip to content

Works without experimental flow #1157

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

Closed
wants to merge 3 commits into from
Closed

Conversation

mjmahone
Copy link
Contributor

For packages that do not use experimental.const_params in their .flowconfig, using v0.12.0 will cause them to have flow errors, as they treat arguments to functions as non-const. This fixes up all of the known cases.

To test, comment-out experimental.const_params=true from .flowconfig, and run flow.

…ms, %checks functions cannot themselves call any functions, and function args are presumed to be non-const. This makes changes to re-assure flow, when being used without const_params, that everything is still safe.
// experimental.const_params=true is set.

// isNamedType(type)
type !== null &&
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!!type && prettiers to Boolean(type) && which is a function call which makes flow fail without experimental.const_params=true. And type && fails because it's not returning a boolean.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can probably Boolean wrap the whole expression then. But also it would be great to avoid the additional instanceof checks - we've managed to corral them into their individual predicate functions so far

leebyron added a commit that referenced this pull request Dec 17, 2017
Merges #1157

Codebases which don't enable this experiment show flow errors
@leebyron
Copy link
Contributor

I just edited this slightly in #1160 to also remove the flow option so this won't regress, I think it can also avoid needing to inline the other checks

leebyron added a commit that referenced this pull request Dec 17, 2017
Merges #1157

Codebases which don't enable this experiment show flow errors
leebyron added a commit that referenced this pull request Dec 17, 2017
Merges #1157

Codebases which don't enable this experiment show flow errors
leebyron added a commit that referenced this pull request Dec 17, 2017
Merges #1157

Codebases which don't enable this experiment show flow errors
@leebyron leebyron deleted the without-experimental-flow branch December 17, 2017 02:29
Copy link

@billydye1 billydye1 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did I do it right

// experimental.const_params=true is set.

// isNamedType(type)
type !== null &&

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fvchbvgg

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

Successfully merging this pull request may close these issues.

4 participants