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

Native String#match declaration incorrect #2450

Closed
milesj opened this issue Sep 13, 2016 · 9 comments
Closed

Native String#match declaration incorrect #2450

milesj opened this issue Sep 13, 2016 · 9 comments
Labels
Library definitions Issues or pull requests about core library definitions

Comments

@milesj
Copy link
Contributor

milesj commented Sep 13, 2016

The declaration here is incorrect:

match(regexp: string | RegExp): ?Array<string>;

According to MDN, match returns an array of strings as well as an index and input property.

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/String/match

@mroch mroch added the Library definitions Issues or pull requests about core library definitions label Sep 16, 2016
@alecperkins
Copy link

I just ran into this issue. It causes flow to fail at usage of the match_instance.index property:

return `${ source.substring(0, ext_match.index) }.css`;
                                         ^^^^^ property `index`. Property not found in
return `${ source.substring(0, ext_match.index) }.css`;
                               ^^^^^^^^^^^^^^^ Array

michelle pushed a commit to michelle/flow that referenced this issue Dec 13, 2016
- The current declaration of String#match fails when trying to access the 'index' and 'input' properties of the return value. (facebook#2450)
michelle added a commit to michelle/flow that referenced this issue Dec 13, 2016
- The current declaration of String#match fails when trying to access the 'index' and 'input' properties of the return value. (facebook#2450)
@miklass
Copy link

miklass commented Feb 8, 2017

Any progress? I've had to do some wacky "any" workarounds and the whole thing felt like a big hack.

@milesj
Copy link
Contributor Author

milesj commented Feb 8, 2017

@miklass I was actually looking into this this week. However, it seems they don't review PRs often (I have another waiting).

@miklass
Copy link

miklass commented Feb 8, 2017

@milesj embrace the hacks :) I've encountered other issues with definitions and behaviors, for example Buffer.isBuffer in node acts like a type check but flow doesn't . Since I'm writing JS code and using the flow comment syntax, it's surprisingly easy to sprinkle lots of "any" cast comments that are blown away in minification step. It's not a great solution, but it allows me to move past the deficiencies in the flow type definitions and focus on real issues in my code

@rstuven
Copy link

rstuven commented Feb 8, 2017

In the meantime, I suggest you add the following to .flowconfig

[options]
suppress_comment=.*\\$FlowIssue

And then, just before the problematic line, add a comment like this:

const rxa = ''.match(/./);
if (rxa) {
  // $FlowIssue: https://github.com/facebook/flow/issues/2450
  console.log(rxa.index);
}

@milesj
Copy link
Contributor Author

milesj commented Feb 9, 2017

PR: #3359

@michaelhogg
Copy link

Since PR #3359 (9611080) was reverted in commit 3107fb3, I think this issue needs to be reopened.

Also see:

@kesor
Copy link

kesor commented Jan 22, 2018

@miklass @milesj I have a hack that does not involve sprinkling any everywhere, just correcting the signature locally using flow-typed/node.js, explained it here - https://stackoverflow.com/questions/48063016/how-to-extend-flow-bin-to-support-new-node-js-methods/48384325

@KevinGrandon
Copy link
Contributor

I believe that this can now be closed due to #6790. @asolove @mrkev Could you help to close this issue?

@mrkev mrkev closed this as completed Sep 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Library definitions Issues or pull requests about core library definitions
Projects
None yet
Development

No branches or pull requests

10 participants