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

fix(utils/normalizeFallback): fallback options behaviour (options.fallback) #145

Merged
merged 1 commit into from Aug 17, 2018

Conversation

alexander-akait
Copy link
Member

@alexander-akait alexander-akait commented Aug 16, 2018

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

fixes #144 fixes #146

Breaking Changes

no

Additional Info

Refactor tests + more tests

exports[`fallback option {String} (with query) 1`] = `"module.exports={\\"limit\\":-9007199254740991,\\"name\\":\\"fallback-[hash].[ext]\\",\\"unknown\\":\\"fallback-value\\"}"`;

exports[`fallback option {String} 1`] = `"module.exports={\\"limit\\":-9007199254740991,\\"name\\":\\"[name].[hash].[ext]\\",\\"unknown\\":\\"value\\"}"`;

Copy link
Member Author

Choose a reason for hiding this comment

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

What we do:

  1. Get original options from url-loader.
  2. Get fallback options.
  3. Merge their : options = Object.assign({}, originalOptions, fallbackOptions);.
  4. delete options.fallback.

@alexander-akait alexander-akait added this to the 1.1.1 milestone Aug 16, 2018
@michael-ciniawsky michael-ciniawsky changed the title fix: fallback options behaviour fix(index): fallback options behaviour (options.fallback) Aug 16, 2018
Copy link
Member

@michael-ciniawsky michael-ciniawsky left a comment

Choose a reason for hiding this comment

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

👍

return normalizeFallbackString('file-loader', originalOptions);
if (index >= 0) {
loader = fallback.substr(0, index);
options = loaderUtils.parseQuery(fallback.substr(index));
Copy link
Member

Choose a reason for hiding this comment

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

parseQuery 👍

@michael-ciniawsky michael-ciniawsky changed the title fix(index): fallback options behaviour (options.fallback) fix(utils/normalizeFallback): fallback options behaviour (options.fallback) Aug 16, 2018
@michael-ciniawsky
Copy link
Member

@Pimm @insin A second pair of eyes welcome please review when time :)

@alexander-akait
Copy link
Member Author

alexander-akait commented Aug 16, 2018

@michael-ciniawsky I think pass all options is the right solution. Also we provide way to rewrite name for fallback loader

@michael-ciniawsky
Copy link
Member

michael-ciniawsky commented Aug 16, 2018

Agreed I just thought passing only the custom options defined via options.fallback.options was an initial requirement for custom fallbacks (I might be totally wrong here)

@alexander-akait
Copy link
Member Author

@michael-ciniawsky let's merge this and release new version

@alexander-akait alexander-akait merged commit 03e631f into master Aug 17, 2018
@alexander-akait alexander-akait deleted the fix-fallback-options-behaviour branch August 17, 2018 09:51
@michael-ciniawsky michael-ciniawsky removed this from the 1.1.2 milestone Aug 17, 2018
@michael-ciniawsky
Copy link
Member

Released in v1.1.2 🎉

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.

Issue with default fallback options Need for utils/normalizeFallback
2 participants