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

Remove redux dependencies from JSON Forms #1649

Merged
merged 26 commits into from
Nov 12, 2020

Conversation

TheZoker
Copy link
Contributor

@TheZoker TheZoker commented Oct 7, 2020

Removes redux as a direct dependency.

  • Redux is removed from the material package (not needed anyway)
  • The missing types have been replaced with own types
  • jsonformsRecducer was removed from the core package (not used anyway)
  • All material tests have been rewritten to run without redux
  • react-redux was moved from the peerDepedencies to the optionalPeerDependencies
  • The jsonFormsReduxContext is now available under @jsonforms/react/redux
import { jsonFormsReduxContext } from `@jsonforms/react/lib/redux`;

Related issue: #1537

Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

I tried it with the React seed and was successfully able to compile and run JSON Forms without installing Redux 🎉

However I was not able to run the fallback Redux variant as the jsonformsReducer is now missing. Please check whether we can restore this export.

Please also check:

  • All tests should run. Currently the core tests (and maybe more) are failing.
  • All example apps should run. Currently at least the React-Material example app doesn't start.

packages/core/src/util/renderer.ts Outdated Show resolved Hide resolved
packages/core/src/util/type.ts Show resolved Hide resolved
packages/core/src/util/type.ts Outdated Show resolved Hide resolved
packages/material/package.json Outdated Show resolved Hide resolved
packages/material/test/renderers/util.ts Outdated Show resolved Hide resolved
packages/react/package.json Outdated Show resolved Hide resolved
packages/core/src/reducers/index.ts Outdated Show resolved Hide resolved
@sdirix sdirix added the hacktoberfest-accepted https://hacktoberfest.digitalocean.com/ label Oct 13, 2020
@TheZoker TheZoker requested a review from sdirix October 15, 2020 10:48
Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

Looks good! Some minor comments.

packages/core/src/util/type.ts Show resolved Hide resolved
packages/core/src/util/type.ts Outdated Show resolved Hide resolved
packages/core/src/reducers/index.ts Outdated Show resolved Hide resolved
@TheZoker TheZoker requested a review from sdirix October 28, 2020 15:44
Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

Build is failing

@coveralls
Copy link

coveralls commented Nov 4, 2020

Coverage Status

Coverage decreased (-0.4%) to 88.383% when pulling 4f2c970 on TheZoker:remove-redux into 0608fff on eclipsesource:master.

Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

The typings for the jsonFormsReducerConfig fail when trying to use a built version. It seems each of the reducers should declare an appropriate Reducer<State,Action> type instead of relying on automatic type conversion.

@TheZoker TheZoker requested a review from sdirix November 11, 2020 11:19
Copy link
Member

@sdirix sdirix left a comment

Choose a reason for hiding this comment

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

LGTM

@sdirix sdirix merged commit d97a584 into eclipsesource:master Nov 12, 2020
@TheZoker TheZoker deleted the remove-redux branch November 17, 2020 13:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted https://hacktoberfest.digitalocean.com/
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants