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

Flow doesn't respect interface definition when implementation exists #676

Closed
skevy opened this issue Jul 30, 2015 · 5 comments
Closed

Flow doesn't respect interface definition when implementation exists #676

skevy opened this issue Jul 30, 2015 · 5 comments

Comments

@skevy
Copy link

skevy commented Jul 30, 2015

I was trying to type check some code that uses Immutable.JS and this interface definition: https://gist.github.com/kastermester/ad79da3e48064effd82e

I was getting stumped when this code wasn't throwing any Flow errors:

import { fromJS, Map as IMap } from 'immutable';

export async function createClientIdAndSecret() : Promise<IMap<string, string>> {
  const clientId: string = uuid.v1();
  const clientSecret: string = crypto.randomBytes(48).toString('hex');

  const shasum = crypto.createHash('sha512');
  shasum.update(clientSecret);
  const clientSecretToStore = shasum.digest('hex');

  await OAuthClient.forge({
    client_id: clientId,
    client_secret: clientSecretToStore
  }).save();

  return 'hello';
}

But this code was throwing the correct error (that "number" and "string" weren't compatible):

import { fromJS, Map as IMap } from 'immutable';

export async function createClientIdAndSecret() : Promise<number> {
  const clientId: string = uuid.v1();
  const clientSecret: string = crypto.randomBytes(48).toString('hex');

  const shasum = crypto.createHash('sha512');
  shasum.update(clientSecret);
  const clientSecretToStore = shasum.digest('hex');

  await OAuthClient.forge({
    client_id: clientId,
    client_secret: clientSecretToStore
  }).save();

  return 'hello';
}

After discussing with @jeffmo on IRC, it seems that the reason this is happening is because Flow can't reason about the Immutable.js build properly, and since it isn't tagged with @flow, treats it's exports as any, and ignores the immutable library def.

Sure enough, when I ignore node_modules/immutable in my .flowconfig, it uses my Immutable interface definition and throws errors (seemingly correct ones too!).

@jeffmo
Copy link
Contributor

jeffmo commented Jul 30, 2015

cc @bhosmer

@Macil
Copy link
Contributor

Macil commented Jul 30, 2015

Seems like a duplicate of #447

@unscriptable
Copy link

Any idea if/when this will be fixed?

@Macil
Copy link
Contributor

Macil commented Feb 5, 2016

I only today discovered that Flow actually supports type-checking of props passed to React components, but you have to add a line like .*/node_modules/react\(-dom\)?/.* to your .flowconfig [ignore] to make it work, because otherwise the React library (without type definitions) overrides Flow's built-in React definitions. I'm really surprised there's such a useful feature hidden by an issue like this.

@JoeStanton
Copy link

I can't get this working, even with the above line in .flowconfig

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

No branches or pull requests

5 participants