Skip to content
This repository has been archived by the owner on Mar 17, 2021. It is now read-only.

feat(index): support fallback loader options (options.fallback) #123

Merged
merged 1 commit into from Jun 16, 2018
Merged

Conversation

Pimm
Copy link
Contributor

@Pimm Pimm commented Apr 6, 2018

It is now possible to explicitly specify options for the fallback loader.

The fallback option now behaves similar to module.rules.use.

This 1.0.1-based configuration…

{
  test: /\.(?:woff|woff2|eot|ttf|svg)$/,
  use: {
    loader: 'url-loader',
    options: {
      limit: 8192,
      fallback: 'file-loader'
    }
  }
}

…can now be written as

{
  test: /\.(?:woff|woff2|eot|ttf|svg)$/,
  use: {
    loader: 'url-loader',
    options: {
      limit: 8192,
      fallback: {
        loader: 'file-loader'
      }
    }
  }
}

And as one would expect, that means options can be provided, as well…

{
  test: /\.(?:woff|woff2|eot|ttf|svg)$/,
  use: {
    loader: 'url-loader',
    options: {
      limit: 8192,
      fallback: {
        loader: 'file-loader',
        options: {
          name: '[name].[ext]'
        }
      }
    }
  }
}

Please look for potential to break existing configurations and surrounding loaders.

@jsf-clabot
Copy link

jsf-clabot commented Apr 6, 2018

CLA assistant check
All committers have signed the CLA.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Very good work! Thanks! Let's add more tests 👍

options: {
limit: 100,
fallback: 'file-loader',
name: '[name].[ext]',
Copy link
Member

Choose a reason for hiding this comment

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

Let's add tests where name is [hash].[ext] to ensure options will send in fallback.

@@ -0,0 +1,48 @@
function normalizeFallbackString(fallbackString, originalOptions) {
Copy link
Member

Choose a reason for hiding this comment

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

Need tests

@@ -0,0 +1,48 @@
function normalizeFallbackString(fallbackString, originalOptions) {
const index = fallbackString.indexOf('?');
if (index >= 0) {
Copy link
Member

Choose a reason for hiding this comment

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

Need tests

@Pimm
Copy link
Contributor Author

Pimm commented Apr 6, 2018

I've added some tests, @ekulabuhov. Please check it out.

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Good work! Thanks!

@michael-ciniawsky michael-ciniawsky changed the title feat: explicit fallback options (#118) feat(index): support fallback loader options (options.fallback) May 5, 2018
Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Sorry for big delay, a lot of work, can you add comments in source why we do delete?

src/index.js Outdated
const fallbackLoaderContext = Object.assign({}, this, {
query: fallbackQuery,
});
delete fallbackLoaderContext.options;
Copy link
Member

Choose a reason for hiding this comment

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

Why we remove fallbackLoaderContext.options?

@Pimm
Copy link
Contributor Author

Pimm commented Jun 5, 2018

@evilebottnawi No problem. I've added a few comment lines. Let me know whether this works for you.

@@ -0,0 +1,48 @@
function normalizeFallbackString(fallbackString, originalOptions) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this to src/utils/normalizeFallback.js, in near future we will release new major version url-loader with dropping node@4 and merge this PR. Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's been moved!

Copy link
Member

Choose a reason for hiding this comment

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

👍

It is now possible to explicitly specify options for the fallback loader.

The new definition (schema) for the fallback option is a lighter variant of the one for module.rules.use. See schemas/WebpackOptions.json in the webpack repository.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants