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

Add ability to declare module interfaces by path #24

Closed
thomasboyt opened this issue Nov 18, 2014 · 9 comments
Closed

Add ability to declare module interfaces by path #24

thomasboyt opened this issue Nov 18, 2014 · 9 comments
Assignees
Labels
incompleteness Something is missing Needs docs

Comments

@thomasboyt
Copy link

Based on conversations in IRC, it seems like the declare module Foo {} syntax for interfaces doesn't support paths. This is bad news for CommonJS, where imports like this are common:

var MyMixin = require('some-module/mixins/MyMixin');

The easiest way to fix this would be to allow a string in the declare module syntax, like:

declare module "some-module/mixins/MyMixin" {}

The same could work for relative imports (e.g. require('../foo/bar'), assuming that Flow can resolve a "full path" from the directory of the file. For example:

// foo/bar/baz.js
var MyMixin = require('../mixins/MyMixin');

should be use the declaration from:

declare module "foo/bar/mixins/MyMixin" {}

assuming that foo is the root of the project.

@gabelevi
Copy link
Contributor

I know there's parser support for this since I added it last night :)

We just need to add support to Flow's core to understand what the path means as a module name.

@avikchaudhuri
Copy link
Contributor

Will fix.

@Raynos
Copy link
Contributor

Raynos commented Nov 19, 2014

👍 Looking forward to this for my commonJS modules

@avikchaudhuri avikchaudhuri added the incompleteness Something is missing label Nov 19, 2014
@avikchaudhuri
Copy link
Contributor

Huh, I just checked what we have, and it seems that at least path-based requires that don't start with /, ./, or ../ should already work with declare module "..." syntax (this is more of a documentation issue, we should explain how this works!)

In other words, you can do exactly what you suggest in the first case, and it should work.

The other cases definitely do not work currently with declare module, since we don't know what a canonical string at the declaration site should be. You suggestion of going relative to the root is a good idea, let me think about it a bit more.

@Raynos
Copy link
Contributor

Raynos commented Nov 20, 2014

I would like to recommend that

declare module "foo/bar/mixins/MyMixin" {}

is relative not to the CWD but to the location of the declaration file.

So if the declaration file is in the foo folder it would be

declare module "bar/mixins/MyMixin" {}

This is consistent with the node lookup algorithm as well for local files.

@avikchaudhuri avikchaudhuri self-assigned this Nov 20, 2014
@Raynos
Copy link
Contributor

Raynos commented Nov 24, 2014

@avikchaudhuri

This is issue is important for being able to use and test flow. I would like to use flowtype with pure ES5 and have local declaration files that define the types of local files.

This would allow me to use flowtype on a non-trivial project without having to use the inline type signatures extended ES5 language.

I would not mind contributing a fix for this if you could point at the rough area of the code that needs to change for this.

@ghost
Copy link

ghost commented Aug 4, 2015

Thank you for reporting this issue and appreciate your patience. We've notified the core team for an update on this issue. We're looking for a response within the next 30 days or the issue may be closed.

@nmn
Copy link
Contributor

nmn commented Mar 4, 2017

This has long been fixed with type imports etc.

@nmn nmn closed this as completed Mar 4, 2017
@capi1O
Copy link

capi1O commented Nov 10, 2017

Sorry @nmn but could you give an example ?

I am in the situation where there I need to require a module by path, and I cannot get Flow to understand the declared types for this module default export.

  • module.js:
const FunctionalComponent = (name: string): React$Element<*> => (<button>{name}</button>);

export default FunctionalComponent;
  • app.js:
const FunctionalComponent = require('./FunctionalComponent'); // eslint-disable-line global-require

render(<FunctionalComponent/>, rootElement);

got the error Expected React component instead of CommonJS exports of ".FunctionalComponent"

I tried to declare the module in FunctionalComponent.flow.js in various ways, ex :

  • FunctionalComponent.flow.js :
declare module 'FunctionalComponent'
{
	declare var exports: (name: string) => React.Component;
}

but this file does not seem to be used by flow at all.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
incompleteness Something is missing Needs docs
Projects
None yet
Development

No branches or pull requests

6 participants