Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

added v5 compilation support and deleted depreciation warnings #1454

Merged
merged 3 commits into from
Aug 30, 2020

Conversation

bartdominiak
Copy link
Contributor

#1408

What I did:

  • Deleted depreciation warnings in v5 (mainTemplate.getAssetPath, and getPublicPath)
  • Added backward compatibility with v4

I also consider to put this webpackMajorVersion, as a helper somewhere - is quite repeated few times. Feedback will be needed.

@bartdominiak
Copy link
Contributor Author

bartdominiak commented Jun 2, 2020

It's looks like there is an error on Compilation array length, which is not related to this PR, and it occur on master branch too.

index.js Outdated
});
};

const childCompilationOutputName = webpackMajorVersion === 5
Copy link

Choose a reason for hiding this comment

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

I would invert the logic to support future versions too (and as it is done in other parts of the codebase)

= webpackMajorVersion === 4 ? ... : ...

Copy link
Contributor Author

@bartdominiak bartdominiak Jun 2, 2020

Choose a reason for hiding this comment

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

That was my first thought too, but we'll lose backward compatibility with v3 and lower, and I'm not 100% that we should support them also? And also this Compilation syntax is different only in webpack v5.

Copy link

Choose a reason for hiding this comment

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

That was my first thought too, but we'll lose backward compatibility with v3 and lower, and I'm not 100% that we should support them also? And also this Compilation syntax is different only in webpack v5.

Compatibility is stated by:

  "peerDependencies": {
    "webpack": ">=4.0.0 < 6.0.0"
  },

"webpack": ">=4.0.0 < 6.0.0"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've one more idea to pick major v5 and higher like:

webpackMajorVersion >= 5 ? ... : ...

What do you think?

Copy link

Choose a reason for hiding this comment

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

I was more looking to how it was written in file-watcher-api.js.

module.exports = webpackMajorVersion === 4

@pi0
Copy link

pi0 commented Aug 17, 2020

Hi @jantimon can you please check this PR? 🙏

@jantimon jantimon merged commit 4ae7be8 into jantimon:master Aug 30, 2020
@jantimon
Copy link
Owner

Released as 4.4.0 - sorry for the delay

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants