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 module bundling & ESM runtime error #280

Merged
merged 1 commit into from
Dec 14, 2019
Merged

Fix module bundling & ESM runtime error #280

merged 1 commit into from
Dec 14, 2019

Conversation

alex-ketch
Copy link
Contributor

@alex-ketch alex-ketch commented Dec 13, 2019

Closes #278

I've tested the ESM build locally and confirmed that I no longer see the original issue.
When building for production, the if (process.env.NODE_ENV !== 'production') { is evaluated to false by Webpack and the chunk of code is omitted from output.

The contents of the generated UMD file (redux-toolkit.umd.min.js), hasn't changed, so there shouldn't be a problem there.
However the development build has changed (screenshot attached below).
I believe this is because redux-immutable-state-invariant was not being instantiated before. I might be missing context here, but let me know if the old behaviour was correct and can fix the logic.

Screen Shot 2019-12-13 at 17 11 10

@netlify
Copy link

netlify bot commented Dec 13, 2019

Deploy preview for redux-starter-kit-docs ready!

Built with commit 8ab6778

https://deploy-preview-280--redux-starter-kit-docs.netlify.com

@codesandbox-ci
Copy link

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 8ab6778:

Sandbox Source
nameless-cache-ep7i0 Configuration
blissful-hooks-li7g4 Configuration
rsk-github-issues-example Configuration

@@ -15,19 +15,26 @@ module.exports = {
case 'umd':
delete config.external
config.output.indent = false
config.plugins.push(
config.plugins.unshift(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The plugins were being called after TS compilation, and weren't behaving as desired. I've moved them to the front of the plugin pipeline so that the compiled code takes the modifications into account.

replace({
'// UMD-DEV-ONLY: ': ''
'// UMD-ONLY: ': '',
delimiters: ['', '']
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This replacement was never matching due to it being constrained to word boundaries as explained here.

config.plugins.unshift(
stripCode({
start_comment: 'PROD_START_REMOVE_UMD',
end_comment: 'PROD_STOP_REMOVE_UMD'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This replace instance allows for instantiation of redux-immutable-state-invariant in development builds only

@markerikson
Copy link
Collaborator

Huh. Yeah, that actually looks like the UMD Dev build is currently broken.

Thanks for fixing this!

@markerikson markerikson merged commit 0f4b0e0 into reduxjs:master Dec 14, 2019
@phryneas
Copy link
Member

phryneas commented Jan 2, 2020

As reported in #297, the PROD_START_REMOVE_UMD annotation does not seem to have the intended effect, or rather this should be removed in both prod and dev, as require is not available in a browser enrivonment. @alex-ketch, would you mind to take a second look? :)

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.

ESM module usage causes Uncaught ReferenceError: require is not defined
3 participants