Treat all unknown extensions as resources

[gaearon]

I’m getting a bit tired of extending our whitelist like in #665.
I think we should have a “catch all” loader that uses file-loader for any unknown extensions.

This makes sense because it’s easy to explain: “if you import something that isn’t JS or CSS, it becomes a part of the build, you get its filename, and can do whatever with it”.

I think @dralletje planned to look into this.

[cesarandreu]

Looking at webpack.config.dev.js, I'm wondering: why use url-loader for audio / video, but file-loader for images? In the scenarios I'm most familiar with, you'd usually do the complete opposite of that: you want essential images, like sprite sheets, to load immediately, but sound and video can wait.

I think this using file-loader for unknown resources is a good default.

[gaearon]

Looking at webpack.config.dev.js, I'm wondering: why use url-loader for audio / video, but file-loader for images?

I didn’t really think it through. If you have better ideas for defaults please submit a PR.

[iRoachie]

@gaearon Any word on this? I wanted to use a php file, to ajax from a form. But I have to manually copy it over after the build process since there's no included loader for php

[gaearon]

@iRoachie I think any backend files (like php) should live outside the project folder. The project folder is only meant to be for files related to the front-end client-side app.

[gaearon]

This is a good example of working with backend APIs. It assumes Node but PHP would work similarly.

[iRoachie]

How's this similar to PHP? I'm using the php to send and email when the data from the form is posted. The link you provided assumes using Node and using endpoints to access the data.

[gaearon]

By similar I meant that, like in that tutorial, you can put client and server code in the different folders, and you can use proxy in development to call /mybackend.php which would get resolved to the specified port. In production, you can have index.php serve built index.html.

[iRoachie]

@gaearon I messaged you on twitter

[TheLarkInn]

This is interesting. We are having the same issue with the angular-cli. In my opinion, regardless of "how" you treat the file (emitted with file-loader into dist, or converted to js, etc), my opinion is that you want it managed by webpack in some way or another.

A catchall unknown-source loader feels like it would be pretty beneficial. I wonder if perhaps leveraging the include property on the loader object to pass a Regex of a folder path like ./assets or w/e.

[bebbi]

@fson, @gaearon I can take this if we agree on a few assumptions:

• I don't think we should rely on naming conventions. When loading a .png and .svg works, loading a .bmp should really not feel any different to a developer. Instead, I suggest that we load anything in /src that's not already addressed by another loader.
• @cesarandreu why not preferring the url-loader with 10k limit for unmatched files?
• I don't think there's an explicit "use this loader if no other addresses it", so I'll suggest using a test regex with a negative lookahead. Unfortunately that'll require re-typing all the file extensions handled by other loaders, but it looks like a small price to pay, I can add comments.
• While at it, if everyone agrees I'll move all items that are currently handled by file loader to url-loader as well per @cesarandreu suggestion.

[gaearon]

Let's reopen until follow up comments from #1059 are addressed.

[bebbi]

@gaearon app is broken. When changing regex in default loader per last PR suggestion it captures the .html now. That sounds like more important than the rest.
Would you like a separate PR immediately?

[bebbi]

Some guidance needed to complete PR comments:

In eslint config L49, should it not be [.js, .jsx] anyway?

Add .jsx to L52?

Do we need line 47 at all, or is specifying import/exensions good enough and we just remove this line?

Validation of default loader: As we could have loader regexes in test, include or exclude, I think validation that works now for most changes is more robust than trying to calculate the default matches. E.g. below naive validation for the default loader that's hard-coded to the current setting - it has false negatives but perhaps that's acceptable.

See proposed jest config - not an expert on jest, any advice for a good test?

const defaultExtensions = module.exports.module.loaders[0].exclude.slice(1);
const specificExtensions = module.exports.module.loaders.slice(1).map(
  loader => loader.test);

if (defaultExtensions.join('') !== specificExtensions.join('')) {
  throw new Error('Default loader inconsistent with other loaders.');
}

[fson]

I think relying on the order of loaders to validate them is going to be very brittle. Then instead of breaking the loader config you'll break the validator.

If we think forgetting to add file types to the default file loader is going to be an issue, maybe we should just compose the loader config from separate parts like this:

var loaders = [
  {
    test: /\.(js|jsx)$/,
    loader: 'babel', ...
  },
  {
    test: /\.css$/,
    loader: ...
  },
  ...
];
var defaultLoader = {
  exclude: loaders.map((loader) => loader.test),
  loader: 'url',
};

// Then in the config:
  ...
  loaders: loaders.concat(defaultLoader),

Yes, it does add some indirection, but it's a tradeoff.

[bebbi]

@JSON I was getting started with this approach but then realized this will break when someone adds include/exclude statements to existing or new loaders. In this case, chances are high that the default loader would break and would do so silently. I think the validator idea has the advantage that it serves as a pull linkage - yes it will fail on changes, but most likely failure will be verbose, which will remind the dev to update the loader and/or the validator.

[fson]

Unless we take in account each test, include and exclude regex and validate that the rules don't have overlap with each other (which due to its complexity I'm not suggesting here, although it might be feasible as a separate tool) we can't protect people editing the config from all kinds of errors. We'll have to assume people who are editing the config by hand know what they're doing or at least can fix errors.

The best thing we can do is keep the config as easy to understand and simple as possible. A check at the end of the file that assumes the first loader is always the catch-all loader and has an exclude array with html as the first item and the file extensions from other loaders after that, in the correct order, is not simple or easy to understand. And it will also break for valid changes at which point you'll have to work around it.

I propose we either compose the catch-all rule from the others like I suggested above, or just have big warning comment that you must add new file extensions to the excludes.

Maybe @gaearon has something to say because he had the idea about validating the config in the first place. :)

[gaearon]

If it's too hard don't validate. :-)

[bebbi]

Eh - sorry @fson for calling you json above :-)) you can see my brain's loaded with loaders.
Pushed everything commented so far. Fine to leave it there - if you have any comments on the other points (eslint things I am not sure of) I'm happy to follow through.

Agree with lack of simplicity. But lets not auto-build a loader for the reasons you say, I think it would silently fail on next changes.

Jest - I'll leave the regex verification up to you in this case if you don't mind.

Thanks!

[fson]

Fixed by #1059 & #1077.

[fson]

Turns out our regex in the Jest moduleNameMapper config didn't work as we had thought, which caused issues when JS files with . in the module name were treated as file resources and stubbed (#1145, #1147).

As a stopgap measure, I reverted the Jest config back to the previous regex which whitelists non-JS/CSS extensions separately, but we still need to find a way to get the same behaviour as our Webpack configuration in Jest.

Maybe @cpojer can help us here? We're trying to create a Jest config that matches this Webpack config:

module: {
  loaders: [
    {
      exclude: [
        /\.html$/,
        /\.(js|jsx)$/,
        /\.css$/,
        /\.json$/
      ],
      loader: 'url',
      ...
    },
    {
      test: /\.(js|jsx)$/,
      include: paths.appSrc,
      loader: 'babel',
      ...
    },
    {
      test: /\.css$/,
      loader: 'style!css?importLoaders=1!postcss'
    },
    {
      test: /\.json$/,
      loader: 'json'
    }
  ]
},

Files with .js, .css and .json extensions are handled separately. Everything else is treated as a file – they should be stubbed in Jest.

The first attempt was a regex with a negative lookahead like this:

moduleNameMapper: {
  '^.+\\.(?!(js|jsx|css|json)$)[^\\.]+$': resolve('config/jest/FileStub.js'),
  '^.+\\.css$': resolve('config/jest/CSSStub.js')
}

However, this doesn't work because moduleNameMapper matches the module name, not the absolute file path like Webpack. Because JS files can be imported without specifying the extension, the module name doesn't necessarily end with .js. This caused false positives where imports like import assign from 'lodash.assign' or import a from '../a' were stubbed.

Is it even possible at the moment to replicate this Webpack config in Jest? I think being able to somehow match against the absolute file path would be necessary for that.

[gaearon]

What @cpojer told me:

The idea is to use the new transform field we added in 17

You can run any transformer on any regex

Default is babel-jest for JS

But you can build a sync transformer for css

[cpojer]

Thanks Dan for sharing my messages to you. Yes, the transform field is meant to be used for this. If you guys are building css-jest and scss-jest and all, I'm happy to support this officially in Jest – this was my long term plan so we can also have style information in JS, just like we do in react-native:

<View
  style={{
    color: 'green',
  }}
/>

etc.