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

fixes computed error when get() returns undefined #455

Merged
merged 2 commits into from Mar 22, 2020

Conversation

jchamb
Copy link
Contributor

@jchamb jchamb commented Mar 19, 2020

Fixes #400
@ctrlplusb I was never able to truly track down why the next prop in get() would be missing the newly added model definition, but the reduce would return an undefined which would throw the error. Simply validating existence after the get() made the error go away and still performed the correct binding of the computed.

In the repo that has this issue, I am using a store provided through a package dependency and it does a number of addModel calls directly after one another, but I was never able to fully reproduce the issue consistently.

@codecov
Copy link

codecov bot commented Mar 19, 2020

Codecov Report

Merging #455 into master will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #455   +/-   ##
=======================================
  Coverage   97.72%   97.72%           
=======================================
  Files          20       20           
  Lines         571      572    +1     
  Branches      104      105    +1     
=======================================
+ Hits          558      559    +1     
  Misses         11       11           
  Partials        2        2           
Impacted Files Coverage Δ
src/create-reducer.js 100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 43f18d6...9e14690. Read the comment docs.

@ctrlplusb
Copy link
Owner

Hey @jchamb;

Thanks for this. This seems likely a reasonable temporary fix. I'll make a note to do some more digging into why the undefined case was occurring in the first place.

Appreciate your help!

👍

ctrlplusb
ctrlplusb previously approved these changes Mar 21, 2020
@@ -47,7 +47,8 @@ export default function createReducer(
: stateAfterActions;
if (state !== next) {
computedProperties.forEach(({ parentPath, bindComputedProperty }) => {
bindComputedProperty(get(parentPath, next));
const prop = get(parentPath, next);
if (prop) bindComputedProperty(prop);
Copy link
Owner

Choose a reason for hiding this comment

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

Actually I have approved this PR already, but we should avoid implicit boolean conversion. I would change this to:

if (prop != null) bindComputedProperty(prop);

@ctrlplusb ctrlplusb dismissed their stale review March 21, 2020 02:05

Saw issue after approving.

@jchamb
Copy link
Contributor Author

jchamb commented Mar 21, 2020

@ctrlplusb once I get some extra free time i'll create some repos I can share that mimic my split setup. I tried reproducing directly within a single repo and never could, so think this might be an issue with having my main store coming in as part of a dependancy.

@ctrlplusb ctrlplusb changed the base branch from master to v3.4.0 March 22, 2020 11:51
@ctrlplusb ctrlplusb merged commit eabfa14 into ctrlplusb:v3.4.0 Mar 22, 2020
ctrlplusb pushed a commit that referenced this pull request Oct 14, 2020
* fixes computed error when get() returns undefined

* avoids implicit boolean conversion
ctrlplusb added a commit that referenced this pull request Oct 14, 2020
* Adds Generics API and TypeScript improvements (#457)

* Fix statemapper eating up classes

* Remove scratch file

* Adds support for generic values

* Adds support for generic values

* Adds useLocalStore hook, deprecates createComponentStore (#458)

* Adds useLocalStore hook

* Adds typings for useLocalStore and it returns store now too

* Adds docs for useLocalStore

* Updates website

* Bumps version

* Fixes rollup config

* Bumps version

* Fixes website -> docs -> quick-start.md (#453)

* Fixes computed properties error for dynamically added model (#455)

* fixes computed error when get() returns undefined

* avoids implicit boolean conversion

* Bumps version

* Updates website

* Disables eslint against TypeScript files

* Upgrades dependencies

* Modified the useLocalStore API to include prev state/config and change config into a factory function

* Bumps version

* Upgrade website dependencies

* Upgrades dependencies

* Replaces immer-peasy with patched immer

* Bumps version

* Fixes bug with evolved models when rehydrating from merge or mergeDeep strategy

* Bumps version

* Adds patches to npm bundle

* Bumps version

* Adds a script to be able to run patch of immer correctly

* Bumps version

* Improves error handling of immer patch script

* Bumps version

* Fixes TypeScript state mapper

* Bump version

* Removes depth limit on TypeScript state mapper

* Bumps version

* Upgrades dependencies

* Removes prop-types as a dependency

* Adds deprecation message to createComponentStore

* Bumps version

* Small refactor to persistence implementation

* The store.persist.flush API now returns a Promise

* Improves persist flush API and adds types and docs on the store instance persist APIs.

* Bumps version

* Moves to microbundle as our bundler

* Adds mangled properties for internals

* Bump

* Fixes persist data rehydration on dynamically added models and introduces an API by which to await the rehydration.

* Bumps version

* Moves tests to root of project

* Upgrades dependencies

* Fix invalid properties being exposed via getActions in TypeScript

* Fixes listeners TypeScript mappings and removes restrictions.

* Updates dependencies

* Fix state mapper and add giant model stress test

* Upgrades to prettier 2

* Bumps version

* Fixes package lock and runs prettier 2 again

* Fixes and improvements to the TypeScript typings

* Bumps version

* Updates redux-thunk middleware position to allow user overriding via the store middleware config

* Bumps version

* Fixes typings issue for computed properties and updates docs with known issue on them

* Bumps version

* added correct homepage url (#470)

* refactor(recipesDoc): renamed selector to computed (#466)

I believe the name changed a while ago, but wasn't reflected in this doc.

* Adds known issue about computed property destructuring

* shortened links (#469)

* Improves the persist API docs

* Adds community extensions page

* Adds test to ensure immer Set and Map support works as expected

* Adds unstable_effectOn API

* Upgrades depenendencies

* Upgrades to TypeScript 3.9

* Updates website

* Bumps version

* Upgrade dev deps

* Upgrades to immer@7 with supreme joy

* Upgrades TS deps

* Bumps version

* Fixes broken test for debug helper

* Updates website

* Refactors store helpers to be immutable

* Upgrade dev deps

* Upgrades deps

* Bumps version

* Disable persist for server side rendering

* Upgrades dev dependencies

* Upgrades dependencies

* Ensures that storage engines are lazily accessed

* Big rewrite to the persistences documentation

* Minor fixes to typescript definitions

* Bumps version

* don't persist computed fields (#540)

* Simplifies thunks and effects. No more error handling. Introduces an explicity 'fail' API for thunks.

* Component stores drop support for initialData runtime prop and adds support for runtime modification to injections.

* DevTools are now only enabled by default if not in production

* Bumps version

* A minor update to consuming-state.md (#490)

* Update README.md

* Update README.md

* A minor update to consuming-state.md

Co-authored-by: Sean Matheson <sean@ctrlplusb.com>

* Updates docs per @gamtiq PR#507

* Updates docs per @gamtiq PR#508

* Updates docs per @gamtiq PR#509

* Updates docs per @gamtiq PR#517

* Updates docs per @minnacaptain PR#519

* Fix persist transformers order @czeslaaw #514

* Reintroduces initialData as runtimeModel on createContextStore

* Updates website and improves persist API

* Bumps version

* Updates readme

* Updates readme

* Updates readme

* Updates readme

* Updates readme

* Adds new scheduling and optimizations to persisting

* Updates docs

* Bump version

* Computed properties now act againt their original immutable state

* Updates docs and tests

* Bumps version

* Adds versioning to persist API, and new tutorials to website

* Bumps version

* Removes node 8 from travis build

* Updated listeners to not respond to thunk fail type by default

* Updates docs

* Removes esm build

* v4

* Removes temp file

Co-authored-by: Hua Lu <lemonadedrink@gmail.com>
Co-authored-by: Jake Chamberlain <jake@jchamb.com>
Co-authored-by: Mohamed Nainar <mdnainar9615@gmail.com>
Co-authored-by: Matthias Rougier <51899780+Matt-Swingvy@users.noreply.github.com>
Co-authored-by: mighdoll <lee@underneath.ca>
Co-authored-by: Ahmed Mohammed <amElnagdy@users.noreply.github.com>
jmyrland pushed a commit that referenced this pull request Sep 16, 2022
* Adds Generics API and TypeScript improvements (#457)

* Fix statemapper eating up classes

* Remove scratch file

* Adds support for generic values

* Adds support for generic values

* Adds useLocalStore hook, deprecates createComponentStore (#458)

* Adds useLocalStore hook

* Adds typings for useLocalStore and it returns store now too

* Adds docs for useLocalStore

* Updates website

* Bumps version

* Fixes rollup config

* Bumps version

* Fixes website -> docs -> quick-start.md (#453)

* Fixes computed properties error for dynamically added model (#455)

* fixes computed error when get() returns undefined

* avoids implicit boolean conversion

* Bumps version

* Updates website

* Disables eslint against TypeScript files

* Upgrades dependencies

* Modified the useLocalStore API to include prev state/config and change config into a factory function

* Bumps version

* Upgrade website dependencies

* Upgrades dependencies

* Replaces immer-peasy with patched immer

* Bumps version

* Fixes bug with evolved models when rehydrating from merge or mergeDeep strategy

* Bumps version

* Adds patches to npm bundle

* Bumps version

* Adds a script to be able to run patch of immer correctly

* Bumps version

* Improves error handling of immer patch script

* Bumps version

* Fixes TypeScript state mapper

* Bump version

* Removes depth limit on TypeScript state mapper

* Bumps version

* Upgrades dependencies

* Removes prop-types as a dependency

* Adds deprecation message to createComponentStore

* Bumps version

* Small refactor to persistence implementation

* The store.persist.flush API now returns a Promise

* Improves persist flush API and adds types and docs on the store instance persist APIs.

* Bumps version

* Moves to microbundle as our bundler

* Adds mangled properties for internals

* Bump

* Fixes persist data rehydration on dynamically added models and introduces an API by which to await the rehydration.

* Bumps version

* Moves tests to root of project

* Upgrades dependencies

* Fix invalid properties being exposed via getActions in TypeScript

* Fixes listeners TypeScript mappings and removes restrictions.

* Updates dependencies

* Fix state mapper and add giant model stress test

* Upgrades to prettier 2

* Bumps version

* Fixes package lock and runs prettier 2 again

* Fixes and improvements to the TypeScript typings

* Bumps version

* Updates redux-thunk middleware position to allow user overriding via the store middleware config

* Bumps version

* Fixes typings issue for computed properties and updates docs with known issue on them

* Bumps version

* added correct homepage url (#470)

* refactor(recipesDoc): renamed selector to computed (#466)

I believe the name changed a while ago, but wasn't reflected in this doc.

* Adds known issue about computed property destructuring

* shortened links (#469)

* Improves the persist API docs

* Adds community extensions page

* Adds test to ensure immer Set and Map support works as expected

* Adds unstable_effectOn API

* Upgrades depenendencies

* Upgrades to TypeScript 3.9

* Updates website

* Bumps version

* Upgrade dev deps

* Upgrades to immer@7 with supreme joy

* Upgrades TS deps

* Bumps version

* Fixes broken test for debug helper

* Updates website

* Refactors store helpers to be immutable

* Upgrade dev deps

* Upgrades deps

* Bumps version

* Disable persist for server side rendering

* Upgrades dev dependencies

* Upgrades dependencies

* Ensures that storage engines are lazily accessed

* Big rewrite to the persistences documentation

* Minor fixes to typescript definitions

* Bumps version

* don't persist computed fields (#540)

* Simplifies thunks and effects. No more error handling. Introduces an explicity 'fail' API for thunks.

* Component stores drop support for initialData runtime prop and adds support for runtime modification to injections.

* DevTools are now only enabled by default if not in production

* Bumps version

* A minor update to consuming-state.md (#490)

* Update README.md

* Update README.md

* A minor update to consuming-state.md

Co-authored-by: Sean Matheson <sean@ctrlplusb.com>

* Updates docs per @gamtiq PR#507

* Updates docs per @gamtiq PR#508

* Updates docs per @gamtiq PR#509

* Updates docs per @gamtiq PR#517

* Updates docs per @minnacaptain PR#519

* Fix persist transformers order @czeslaaw #514

* Reintroduces initialData as runtimeModel on createContextStore

* Updates website and improves persist API

* Bumps version

* Updates readme

* Updates readme

* Updates readme

* Updates readme

* Updates readme

* Adds new scheduling and optimizations to persisting

* Updates docs

* Bump version

* Computed properties now act againt their original immutable state

* Updates docs and tests

* Bumps version

* Adds versioning to persist API, and new tutorials to website

* Bumps version

* Removes node 8 from travis build

* Updated listeners to not respond to thunk fail type by default

* Updates docs

* Removes esm build

* v4

* Removes temp file

Co-authored-by: Hua Lu <lemonadedrink@gmail.com>
Co-authored-by: Jake Chamberlain <jake@jchamb.com>
Co-authored-by: Mohamed Nainar <mdnainar9615@gmail.com>
Co-authored-by: Matthias Rougier <51899780+Matt-Swingvy@users.noreply.github.com>
Co-authored-by: mighdoll <lee@underneath.ca>
Co-authored-by: Ahmed Mohammed <amElnagdy@users.noreply.github.com>
jmyrland pushed a commit that referenced this pull request Sep 16, 2022
* Adds Generics API and TypeScript improvements (#457)

* Fix statemapper eating up classes

* Remove scratch file

* Adds support for generic values

* Adds support for generic values

* Adds useLocalStore hook, deprecates createComponentStore (#458)

* Adds useLocalStore hook

* Adds typings for useLocalStore and it returns store now too

* Adds docs for useLocalStore

* Updates website

* Bumps version

* Fixes rollup config

* Bumps version

* Fixes website -> docs -> quick-start.md (#453)

* Fixes computed properties error for dynamically added model (#455)

* fixes computed error when get() returns undefined

* avoids implicit boolean conversion

* Bumps version

* Updates website

* Disables eslint against TypeScript files

* Upgrades dependencies

* Modified the useLocalStore API to include prev state/config and change config into a factory function

* Bumps version

* Upgrade website dependencies

* Upgrades dependencies

* Replaces immer-peasy with patched immer

* Bumps version

* Fixes bug with evolved models when rehydrating from merge or mergeDeep strategy

* Bumps version

* Adds patches to npm bundle

* Bumps version

* Adds a script to be able to run patch of immer correctly

* Bumps version

* Improves error handling of immer patch script

* Bumps version

* Fixes TypeScript state mapper

* Bump version

* Removes depth limit on TypeScript state mapper

* Bumps version

* Upgrades dependencies

* Removes prop-types as a dependency

* Adds deprecation message to createComponentStore

* Bumps version

* Small refactor to persistence implementation

* The store.persist.flush API now returns a Promise

* Improves persist flush API and adds types and docs on the store instance persist APIs.

* Bumps version

* Moves to microbundle as our bundler

* Adds mangled properties for internals

* Bump

* Fixes persist data rehydration on dynamically added models and introduces an API by which to await the rehydration.

* Bumps version

* Moves tests to root of project

* Upgrades dependencies

* Fix invalid properties being exposed via getActions in TypeScript

* Fixes listeners TypeScript mappings and removes restrictions.

* Updates dependencies

* Fix state mapper and add giant model stress test

* Upgrades to prettier 2

* Bumps version

* Fixes package lock and runs prettier 2 again

* Fixes and improvements to the TypeScript typings

* Bumps version

* Updates redux-thunk middleware position to allow user overriding via the store middleware config

* Bumps version

* Fixes typings issue for computed properties and updates docs with known issue on them

* Bumps version

* added correct homepage url (#470)

* refactor(recipesDoc): renamed selector to computed (#466)

I believe the name changed a while ago, but wasn't reflected in this doc.

* Adds known issue about computed property destructuring

* shortened links (#469)

* Improves the persist API docs

* Adds community extensions page

* Adds test to ensure immer Set and Map support works as expected

* Adds unstable_effectOn API

* Upgrades depenendencies

* Upgrades to TypeScript 3.9

* Updates website

* Bumps version

* Upgrade dev deps

* Upgrades to immer@7 with supreme joy

* Upgrades TS deps

* Bumps version

* Fixes broken test for debug helper

* Updates website

* Refactors store helpers to be immutable

* Upgrade dev deps

* Upgrades deps

* Bumps version

* Disable persist for server side rendering

* Upgrades dev dependencies

* Upgrades dependencies

* Ensures that storage engines are lazily accessed

* Big rewrite to the persistences documentation

* Minor fixes to typescript definitions

* Bumps version

* don't persist computed fields (#540)

* Simplifies thunks and effects. No more error handling. Introduces an explicity 'fail' API for thunks.

* Component stores drop support for initialData runtime prop and adds support for runtime modification to injections.

* DevTools are now only enabled by default if not in production

* Bumps version

* A minor update to consuming-state.md (#490)

* Update README.md

* Update README.md

* A minor update to consuming-state.md

Co-authored-by: Sean Matheson <sean@ctrlplusb.com>

* Updates docs per @gamtiq PR#507

* Updates docs per @gamtiq PR#508

* Updates docs per @gamtiq PR#509

* Updates docs per @gamtiq PR#517

* Updates docs per @minnacaptain PR#519

* Fix persist transformers order @czeslaaw #514

* Reintroduces initialData as runtimeModel on createContextStore

* Updates website and improves persist API

* Bumps version

* Updates readme

* Updates readme

* Updates readme

* Updates readme

* Updates readme

* Adds new scheduling and optimizations to persisting

* Updates docs

* Bump version

* Computed properties now act againt their original immutable state

* Updates docs and tests

* Bumps version

* Adds versioning to persist API, and new tutorials to website

* Bumps version

* Removes node 8 from travis build

* Updated listeners to not respond to thunk fail type by default

* Updates docs

* Removes esm build

* v4

* Removes temp file

Co-authored-by: Hua Lu <lemonadedrink@gmail.com>
Co-authored-by: Jake Chamberlain <jake@jchamb.com>
Co-authored-by: Mohamed Nainar <mdnainar9615@gmail.com>
Co-authored-by: Matthias Rougier <51899780+Matt-Swingvy@users.noreply.github.com>
Co-authored-by: mighdoll <lee@underneath.ca>
Co-authored-by: Ahmed Mohammed <amElnagdy@users.noreply.github.com>
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.

addModel and computed props
2 participants