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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move miniKindOf out of if scope to fix ES5 compatibility issue #4090

Merged
merged 1 commit into from May 22, 2021

Conversation

embeddedt
Copy link


name: 馃悰 Bug fix or new feature
about: Fixing a problem with Redux

PR Type

Does this PR add a new feature, or fix a bug?

It fixes a bug.

Why should this PR be included?

Currently redux cannot be run in ES5 environments even if Babel is used.

Checklist

  • Have you added an explanation of what your changes do and why you'd like us to include them?
  • Is there an existing issue for this PR?
  • Have the files been linted and formatted?
  • Have the docs been updated to match the changes in the PR?
  • Have the tests been updated to match the changes in the PR?
  • Have you run the tests locally to confirm they pass?

Bug Fixes

What is the current behavior, and the steps to reproduce the issue?

Please refer to #4089.

TL;DR There are functions declared inside an if statement, which ES5 forbids and Babel does not fix.

What is the expected behavior?

Redux should run without any syntax errors, if it's being bundled and transpiled in the standard manner for ES5.

How does this PR fix the problem?

It moves the aforementioned functions out of the if statement and into the file's scope.

The diff looks huge but it's mostly due to the indentation changing.

@codesandbox-ci
Copy link

codesandbox-ci bot commented May 22, 2021

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 89a516c:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration

@netlify
Copy link

netlify bot commented May 22, 2021

Deploy Preview for redux-docs ready!

Built with commit 89a516c

https://deploy-preview-4090--redux-docs.netlify.app

@markerikson markerikson changed the base branch from master to 4.x May 22, 2021 20:40
@markerikson markerikson changed the base branch from 4.x to master May 22, 2021 20:40
@markerikson
Copy link
Contributor

markerikson commented May 22, 2021

FYI, this really needs to be based against the 4.x branch if we're going to release it. master is an unreleased TypeScript conversion.

Also, I'd like to make sure that the function only actually exists in dev mode to save on byte size, but that itself requires an if(process.env.NODE_ENV === 'development') check. Not sure if there's a good way to resolve that.

@markerikson markerikson changed the base branch from master to 4.x May 22, 2021 20:45
@embeddedt
Copy link
Author

FYI, this really needs to be based against the 4.x branch if we're going to release it. master is an unreleased TypeScript conversion.

Thanks; I've force pushed a version based off 4.x.

Also, I'd like to make sure that the function only actually exists in dev mode to save on byte size, but that itself requires an if(process.env.NODE_ENV === 'development') check. Not sure if there's a good way to resolve that.

Assuming that production environments are using something like terser, wouldn't it automatically see the function as dead code and remove it, since the NODE_ENV check will be replaced by false?

I've tested it locally by replacing the check with 'production' !== 'production' and running npx terser --module -c dead_code src/utils/kindOf.js and it does get eliminated.

@markerikson
Copy link
Contributor

Okay, sounds good. There's a couple other little bits that probably need to go into a 4.1.1, so I'll see if I can get this out in the next couple days.

@markerikson markerikson merged commit 0874b13 into reduxjs:4.x May 22, 2021
timdorr added a commit that referenced this pull request Aug 3, 2021
webMasterMrBin pushed a commit to webMasterMrBin/redux that referenced this pull request Aug 21, 2021
Move miniKindOf out of if scope to fix ES5 compatibility issue
webMasterMrBin pushed a commit to webMasterMrBin/redux that referenced this pull request Aug 21, 2021
webMasterMrBin pushed a commit to webMasterMrBin/redux that referenced this pull request Aug 21, 2021
Move miniKindOf out of if scope to fix ES5 compatibility issue

Former-commit-id: cecf124
webMasterMrBin pushed a commit to webMasterMrBin/redux that referenced this pull request Aug 21, 2021
webMasterMrBin pushed a commit to webMasterMrBin/redux that referenced this pull request Aug 21, 2021
Former-commit-id: 4627a1a
Former-commit-id: 023d9c4
webMasterMrBin pushed a commit to webMasterMrBin/redux that referenced this pull request Aug 21, 2021
Move miniKindOf out of if scope to fix ES5 compatibility issue

Former-commit-id: cecf124
Former-commit-id: 83b2ac1
webMasterMrBin pushed a commit to webMasterMrBin/redux that referenced this pull request Aug 21, 2021
Move miniKindOf out of if scope to fix ES5 compatibility issue

Former-commit-id: cecf124
Former-commit-id: 83b2ac1
webMasterMrBin pushed a commit to webMasterMrBin/redux that referenced this pull request Aug 21, 2021
Former-commit-id: 4627a1a
Former-commit-id: 023d9c4
webMasterMrBin pushed a commit to webMasterMrBin/redux that referenced this pull request Aug 21, 2021
Former-commit-id: 4627a1a
Former-commit-id: 023d9c4
webMasterMrBin pushed a commit to webMasterMrBin/redux that referenced this pull request Aug 21, 2021
Former-commit-id: 4627a1a
Former-commit-id: 023d9c4
webMasterMrBin pushed a commit to webMasterMrBin/redux that referenced this pull request Aug 21, 2021
Move miniKindOf out of if scope to fix ES5 compatibility issue

Former-commit-id: cecf124
Former-commit-id: 83b2ac1
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

2 participants