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

Fix Web Workers compatibility #2624

Merged
merged 2 commits into from
Jul 22, 2021
Merged

Fix Web Workers compatibility #2624

merged 2 commits into from
Jul 22, 2021

Conversation

legendecas
Copy link
Contributor

@legendecas legendecas commented Jun 23, 2021

Latest Webpack generated dist files no longer reference "window". Thus those
generated files are compatible with Web Workers.

Fixes #2251

Latest Webpack generated dist files no longder reference "window". Thus those
generated files are compatible with Web Workers.
@hueniverse
Copy link
Contributor

@Marsup Can you review?

Copy link
Collaborator

@Marsup Marsup left a comment

Choose a reason for hiding this comment

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

We can also get rid of null-loader but I can do it if you don't want to.

Comment on lines 21 to 23
WebpackConfig.plugins.push(new Webpack.ProvidePlugin({
process: 'process/browser',
}));
Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like we can remove process by doing this.

Suggested change
WebpackConfig.plugins.push(new Webpack.ProvidePlugin({
process: 'process/browser',
}));
WebpackConfig.plugins.push(new Webpack.DefinePlugin({
'process.env.NODE_DEBUG': false,
}));

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated :)

@legendecas
Copy link
Contributor Author

legendecas commented Jul 5, 2021

We can also get rid of null-loader but I can do it if you don't want to.

@Marsup That doesn't seem to be related to the web compatibility issue that fixed in this PR. I have no idea about the context of that code piece. It will be helpful if you can take that part of the work.

@hueniverse
Copy link
Contributor

@Marsup Leaving this for you to merge.

@Marsup Marsup self-assigned this Jul 22, 2021
@Marsup
Copy link
Collaborator

Marsup commented Jul 22, 2021

I'm merging this as is and will bring some modifications afterwards. @hueniverse I'll leave the version numbering to you, I'm not sure which applies in this case.

@Marsup Marsup merged commit 39d314d into hapijs:master Jul 22, 2021
Marsup added a commit that referenced this pull request Jul 22, 2021
@legendecas legendecas deleted the webpack branch July 23, 2021 02:48
@hueniverse
Copy link
Contributor

@Marsup I assume this is not a breaking change?

@Marsup
Copy link
Collaborator

Marsup commented Jul 28, 2021

Unlikely, minor at best.

@hueniverse hueniverse added the bug Bug or defect label Aug 1, 2021
@hueniverse hueniverse added this to the 17.4.2 milestone Aug 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug or defect
Projects
None yet
Development

Successfully merging this pull request may close these issues.

dist/joi-browser.min.js doesn't work in workers
3 participants