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

feat: Apply webpack-defaults v1.x #165

Closed
wants to merge 8 commits into from
Closed

Conversation

joshwiens
Copy link
Member

What kind of change does this PR introduce?

Refactoring

Did you add tests for your changes?

If relevant, did you update the README?

Summary

  • Retains index.js file history
  • Assumes proposed change to npm@5
  • Assumes proposed appveyor proposal

Can't rebase in the changes from master given the index.js in the old defaults upgrade was deleted.

Has the latest proposed changes to defaults ( appveyor & npm@5 )

Does this PR introduce a breaking change?

Breaking Change - Enforces NodeJS > 4.3 via engines.

Must release as semver MAJOR

Other information

 - Retains index.js file history
 - Assumes proposed change to npm@5
 - Assumes proposed appveyor proposal
@joshwiens joshwiens added this to the 1.0.0 milestone Jun 5, 2017
src/index.js Outdated
import loaderUtils from 'loader-utils';

export default function fileLoader(content) {
this.cacheable && this.cacheable(); // eslint-disable-line no-unused-expressions
Copy link
Member

@michael-ciniawsky michael-ciniawsky Jun 5, 2017

Choose a reason for hiding this comment

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

This isn't needed with webpack >= v2.0.0, I know the PR 'just' applies webpack-defaults for the moment, so could be done later, but it's a simple change so maybe :)

Copy link
Member Author

Choose a reason for hiding this comment

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

No, you're right about that. File loader was the guinea pig for defaults. It's being redone only to retain the git history on the lib files and to save me the trouble of having to rebase in changes on a file that was deleted on this branch :)


const filePath = this.resourcePath;
if (config.useRelativePath) {
const issuerContext = this._module && this._module.issuer // eslint-disable-line no-underscore-dangle
Copy link
Member

Choose a reason for hiding this comment

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

Maybe we should relax some of these ESLint rules or are these lines subject to change in a smaller upcoming PR by intention ?

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.

Nevermind I saw the v1.1.0 milestone 😛

@joshwiens
Copy link
Member Author

The CI problem is a linux only thing, i'll figure it out this afternoon.

@joshwiens joshwiens force-pushed the d3viant0ne-Defaults branch from 0c46ff9 to 68ab09f Compare June 7, 2017 16:26
@joshwiens joshwiens closed this Jun 7, 2017
@joshwiens joshwiens deleted the d3viant0ne-Defaults branch June 7, 2017 20:40
@michael-ciniawsky michael-ciniawsky removed this from the 3.0.0 milestone Aug 21, 2018
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.

2 participants