Fix #201: Enforce -loader suffix when processing loaders to handle old & new Webpack compatibility

[kevinzwhuang]

Summary:

The latest build of Webpack (v2.1.0-beta.26) requires loaders be explicitly stated with the -loader suffix, thus causing bootstrap-loader to no longer be compatible due to use of shorthand loader names.

This PR fixes #201.

Fix Proposed:

This PR handles both cases, with or without -loader, to keep compatibility with old webpack users that omit the -loader suffix. To do so, I'm proposing the following changes:

• Style loaders coming from .bootstraprc are checked for a -loader suffix using regex to find for compatible style loaders.
• If the style loader coming from .bootstraprc is missing the -loader suffix, it replaces the short name with the name + -loader. Otherwise, it returns the original loader name if it passes the suffix test.
• For buildExtractStyleLoaders, the fallback loader now comes with the -loader suffix. Kudos to @silveryiris for this original solution. see Webpack v2.1.0-beta.26 - Breaking changes with loaders #201 (comment)

Test Plan:

buildExtractStylesLoader.test.js and processStyleLoaders.test.js have both been updated to reflect these new changes and test correctly for the new -loader suffix. This should pass the requirement in Webpack v2.1.0-beta.26.

Some additional configuration work will be required to bring the examples up to par with beta.26, however to keep the scope of this PR controlled to the functionality of -loader suffix handling, that work is not included in this PR. I can follow up with another PR for proper beta.26 configuration for the examples.

---

cc: @justin808

---

This change is 

[justin808]

1. Please include a CHANGELOG entry for this one. 2. We need to remove any restrictions on the upper bound for webpack from the examples directory.

@Judahmeek, please review.

---

Reviewed 4 of 4 files at r1.
Review status: all files reviewed at latest revision, 1 unresolved discussion.

---

src/utils/processStyleLoaders.js, line 27 at r1 (raw file):

    }
    return loader;
  });

How about putting all of the above into a separate function, taking one param, loaders?

---

Comments from Reviewable

[kevinzwhuang]

@justin808
Made a few updates to the PR to get it ready.

• Moved the suffix ensurement logic inside a separate function, as you suggested.
• I added an entry to the CHANGELOG under Unreleased, hope that works.
• Also given that the we're removing the upper bound for the examples, I went ahead and made sure they were all compliant with the -loader suffix for all loaders in their webpack.config files.
• Given that we're removing the upperbound for the examples, I also removed the upper bound from the main package.json - I think that change makes sense for now.
• Lastly, I did a once-over through the README and found every example of webpack loaders and added the -loader suffix to help with the transition for future users - in addition to a note about the automatic suffix ensurement for styleLoaders.

[justin808]

@Judahmeek Any chance that you can verify all the examples work with the latest webpack?
@kevinzwhuang Please confirm you tried all the examples with both bootstrap 3 and 4.

@robwise @alexfedoseev We should use this in F&G.

---

Reviewed 13 of 13 files at r2.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

src/utils/processStyleLoaders.js, line 20 at r2 (raw file):

  // Enforce -loader suffix for loaders in config for webpack compatibility
  const ensureLoadersSuffix = loadersArray => {

I would put this as a function above the default export. I don't think having an inline function makes this more readable. What's your opinion?

---

Comments from Reviewable

[kevinzwhuang]

@justin808, About moving the function above the default export for readability - makes sense, I agree. It's a bit verbose to be inline. I'll update this branch accordingly.

I manually ran all the examples last night, seems to work for basic & css-modules - dev & prod versions. I think there's an error with the multiple-entries example, I'll take a closer look in a moment. Will report back once working.

[kevinzwhuang]

@justin808 Since postcss is used in all of the bootstrap config examples for BS4, I added the postcss-loader pattern to one of the loaders ensured a -loader suffix by the ensureLoadersSuffix regex logic.

Also, I updated all of the example bootstrap config files to use the -loader suffix explicitly as well. I tested all examples with bs3/bs4 + dev/prod + longform/shortform named styleLoaders - all working!

[nappynapster]

Hey,
when do you think this PR will be merged/ready?

[kevinzwhuang]

@nappynapster It's pretty much ready now, just need @justin808 and @Judahmeek to verify that it's ready to be merged in.

[justin808]

I'll release. @nappynapster Can you also verify the examples are working?

---

Reviewed 7 of 7 files at r3.
Review status: all files reviewed at latest revision, 2 unresolved discussions.

---

Comments from Reviewable

[justin808]

Released 2.0.0-beta.15.

@alexfedoseev @robwise you probably want this in https://www.friendsandguests.com.

[kevinzwhuang]

Great, thanks @justin808!