Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Support Webpack 5 by updating VirtualModulesPlugin from upstream #132

Closed
wants to merge 0 commits into from

Conversation

syvb
Copy link
Contributor

@syvb syvb commented Sep 8, 2020

tl;dr: Webpack 5 support, fixes TypeError: Cannot read property 'data' of undefined

This updates the internal VirtualModulesPlugin, which is copied from webpack-virtual-modules, which was originally added by @kaisermann. These changes allow for using Webpack 5 with svelte-loader. The changes of #125, which have also been fixed upstream, have been preserved. See #59 (comment) for the background on why the source code is just copied over instead of added as a dependency. Due to the svelte-loader specific changes already in virtual.js, I have manually gone through the commit history of webpack-virtual-modules and applied relevant changes instead of just copy-pasting the new source code. There is also a small change to index.js to prevent the error handling code from itself throwing errors, since that was causing some (now fixed) troubles when debugging my changes.

This also adds in the literal text of the license of webpack-virtual-modules, since "The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software".

Tests and lints pass, and I have verified this works with both Webpack 4 and Webpack 5.

@benmccann
Copy link
Member

For the license, let's just create a LICENSE file in the repo like https://github.com/sveltejs/svelte/blob/master/LICENSE instead of adding to each individual file.

lib/virtual.js Outdated Show resolved Hide resolved
@syvb
Copy link
Contributor Author

syvb commented Sep 9, 2020

@benmccann I've added a LICENSE file to the root, and updating the directory handling code with the changes from 2773097.

Copy link
Member

@benmccann benmccann left a comment

Choose a reason for hiding this comment

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

thanks. this seems fine to me. I'll leave it for someone who's more familiar with this repo to do the merge and cut a release

@non25
Copy link
Contributor

non25 commented Sep 14, 2020

While changes from this PR hasn't been merged I ask you to integrate an important fix from upstream.
sysgears/webpack-virtual-modules@0c8926e

Context:
sysgears/webpack-virtual-modules#65
sysgears/webpack-virtual-modules#66
#131
#133

@syvb
Copy link
Contributor Author

syvb commented Sep 19, 2020

@non25 I've updated with that patch. Not sure why this got closed.

@benmccann
Copy link
Member

@Smittyvb it says there are no code changes on the branch anymore, which is why I think it was closed

@syvb
Copy link
Contributor Author

syvb commented Sep 19, 2020

@benmccann It looks like GH will no longer let me re-open this, so see #136 for the continuation of this.
image

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

Successfully merging this pull request may close these issues.

None yet

3 participants