Skip to content
This repository was archived by the owner on Dec 19, 2023. It is now read-only.

Conversation

@elifitch
Copy link
Contributor

@elifitch elifitch commented Jun 3, 2019

This upgrades to webpack 4.28.2 and pegs us at that version for the time being. The problem here is that newer versions of webpack have hard-to-diagnose problems with dynamic imports. Per this issue webpack/webpack#8656 there are workarounds one can take at the project level, and for some, using yarn instead of npm has rectified the problem. This is kind of a blanket approach, as it just pegs us at the version before this problem was introduced.

I didn't write any tests to confirm that dynamic imports work, as it was a bit difficult for me to come to grips with the underreact test suite. I did however include a dynamic import in the fancy/ example.

Either @kepta or @davidtheclark for review ❤️

@elifitch elifitch requested review from davidtheclark and kepta June 3, 2019 17:48
@@ -0,0 +1,5 @@
function dynamic() {
console.log('hi i am a dynamic import');
Copy link
Contributor

Choose a reason for hiding this comment

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

@elifitch instead of console logging can we show a UI change in the ./fancy/src/app.js component.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 👍

Copy link
Contributor

@kepta kepta left a comment

Choose a reason for hiding this comment

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

Looks great 👍 , thanks for fixing this.

@elifitch
Copy link
Contributor Author

elifitch commented Jun 3, 2019

@kepta how should we go about merging and releasing this?

@kepta
Copy link
Contributor

kepta commented Jun 19, 2019

@elifitch you can merge it and then:

  • update the changelog
  • npm publish a minor or patch release
  • git push to master with tags

@elifitch elifitch merged commit e8a87bf into master Jun 19, 2019
@elifitch elifitch deleted the dynamic-imports branch June 19, 2019 13:16
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants