Skip to content

Builtin support for requiring resource files (css, jpg, etc)#2252

Closed
gabelevi wants to merge 1 commit into
facebook:masterfrom
gabelevi:resource
Closed

Builtin support for requiring resource files (css, jpg, etc)#2252
gabelevi wants to merge 1 commit into
facebook:masterfrom
gabelevi:resource

Conversation

@gabelevi

Copy link
Copy Markdown
Contributor

Often, people require resource files from their JavaScript. Until now, this has
mainly been handled by using the module.name_mapper option in the
./flowconfig like so:

module.name_mapper='^(.*).css$' -> 'flow/css'
module.name_mapper='^(.*).(jpg\|png\|gif\|eot\|svg\|ttf\|woff\|woff2\|mp4\|webm)$' -> 'flow/file'

Where flow/css.js would export a css mock and flow/file.js would export
string. This would let you write

const myCSS = require('./myCSS.css');
const myJPG = require('./myJPG.jpg');

This change teaches Flow that it's ok to import and require these resource
files. By default, requiring a resource file returns a string, except for css
which returns void. This hopefully will let code that requires resource files
to work with Flow without the need for configuration.

Often, people require resource files from their JavaScript. Until now, this has
mainly been handled by using the `module.name_mapper` option in the
`./flowconfig` like so:

```
module.name_mapper='^(.*).css$' -> 'flow/css'
module.name_mapper='^(.*).(jpg\|png\|gif\|eot\|svg\|ttf\|woff\|woff2\|mp4\|webm)$' -> 'flow/file'
```

Where `flow/css.js` would export a css mock and `flow/file.js` would export
`string`. This would let you write

```
const myCSS = require('./myCSS.css');
const myJPG = require('./myJPG.jpg');
```

This change teaches Flow that it's ok to import and require these resource
files. By default, requiring a resource file returns a `string`, except for css
which returns `void`. This hopefully will let code that requires resource files
to work with Flow without the need for configuration.
@gabelevi

Copy link
Copy Markdown
Contributor Author

@facebook-github-bot import

@ghost ghost added the CLA Signed label Aug 15, 2016
@ghost

ghost commented Aug 15, 2016

Copy link
Copy Markdown

Thanks for importing.If you are an FB employee go to Phabricator to review internal test results.

@jamiebuilds

Copy link
Copy Markdown
Contributor

cc @gaearon

let reason, exports_t = match Utils.extension_of_filename filename with
| Some ".css" ->
let reason = Reason.mk_reason
"Flow assumes requiring a .css file returns undefined"

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Many people use CSS modules which export an object with string values for each key. Can we give a hint on how to configure those for people who want them?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Maybe it makes sense to make { [key: string]: any } the default since the people that will care about it will use it that way and the ones who don't won't use it at all.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Agree.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Ah, just saw this. I'll switch to Object by default for .css then.

There was a little disagreement about the configuration, so as a compromise I landed this PR without any new configuration options. We'll figure those out afterwards.

module.name_mapper does still work, though, so if you need something special for css you can still do

[options]
module.name_mapper='^(.*).css$' -> 'flow/css'

Where flow/css.js exports whatever type you'd like.

@ghost ghost closed this in 6984935 Aug 24, 2016
ghost pushed a commit that referenced this pull request Aug 25, 2016
Summary:
Missed the discussion between gaearon and thejameskyle on
#2252, but it seems like a decent default.

Reviewed By: avikchaudhuri

Differential Revision: D3763858

fbshipit-source-id: 87c01333bb039b5ef6feaaba67bd48c6677e2feb
This pull request was closed.
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