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

Introduce *IfNecessary Load Actions #148

Merged
merged 3 commits into from
Sep 2, 2020
Merged

Conversation

jrista
Copy link
Contributor

@jrista jrista commented Aug 30, 2020

feat: add load if necessary action variants

 + Add LoadIfNecessary action
 + Add LoadAllIfNecessary action
 + Add LoadManyIfNecessary action
 + Add LoadPageIfNecessary action
 + Add LoadRangeIfNecessary action
 * Export new actions from public api
 + Add new operators for if necessary loads
 + Add new effects for if necessary loads
 * Include new effects in standard modules
 * Export new effects from public api
 + Add stateNameOfEntity utility function
 * Update builders to use new entityStateName function

Issue #144


feat: add defaultMaxAge to entity operator

 * Introduce to `defaultMaxAge` to IEntityOptions
 + Add EntityAge with predefined common ages for use with `defaultMaxAge`
 * Update @Entity decorator to support passing name in only or options or both
 > Move key decorator tokens into entity-tokens.ts
 > Extract entity decorator into its own file entity-decorator.ts
 > Extract key decorator into its own file key-decorator.ts
 * Update index/public api to reference all exports from new locations
 * Added EntityAge enum to public api
 * Removed all imports from '../..'

Issues #144 #141


feat: reorg action files and introduce *IfNecessary load actions

 * Refactor all actions into more files for easier management
   * Loads into load-*-actions.ts
   * CUURDS into create|update|upsert|replace|delete-actions.ts
   * DeleteByKeys into delete-by-key-actions.ts
   * Edits into edit-actions.ts
   * Selections into selection-actions.ts
   * Deselections into deselection-actions.ts
 + Add new *IfNecessary loader actions for each type of load
 + Add tsdoc to all load actions
 * Update library index/public api to reference all actions from new files
 * Update all otehr action references throughout library
 ^ Bump library version to 0.5.0-beta.3

Phase #1: This is an initial refactoring of action files for the if-necessary work

Issue #144

 * Refactor all actions into more files for easier management
   * Loads into load-*-actions.ts
   * CUURDS into create|update|upsert|replace|delete-actions.ts
   * DeleteByKeys into delete-by-key-actions.ts
   * Edits into edit-actions.ts
   * Selections into selection-actions.ts
   * Deselections into deselection-actions.ts
 + Add new *IfNecessary loader actions for each type of load
 + Add tsdoc to all load actions
 * Update library index/public api to reference all actions from new files
 * Update all otehr action references throughout library
 ^ Bump library version to 0.5.0-beta.3

Phase #1: This is an initial refactoring of action files for the if-necessary work

Issue #144
@jrista
Copy link
Contributor Author

jrista commented Aug 30, 2020

NOTE: DO NOT MERGE!!

This PR is just phase one of the work for this feature. Additional work is being done to integrate the new *IfNecessary actions into the effects. Do not merge until the second phase of the work has been committed.

@jrista jrista requested review from anthonymjones, patpaquette, schuchard and Wells-Codes and removed request for patpaquette August 30, 2020 16:16
@jrista jrista self-assigned this Aug 30, 2020
@jrista jrista force-pushed the feat/nea-144/loads-if-necessary branch 2 times, most recently from 8e4f61b to 98fe940 Compare August 30, 2020 18:04
 * Introduce to `defaultMaxAge` to IEntityOptions
 + Add EntityAge with predefined common ages for use with `defaultMaxAge`
 * Update @entity decorator to support passing name in only or options or both
 > Move key decorator tokens into entity-tokens.ts
 > Extract entity decorator into its own file entity-decorator.ts
 > Extract key decorator into its own file key-decorator.ts
 * Update index/public api to reference all exports from new locations
 * Added EntityAge enum to public api
 * Removed all imports from '../..'

Issues #144 #141
@jrista jrista force-pushed the feat/nea-144/loads-if-necessary branch from 98fe940 to 9ba68f8 Compare August 30, 2020 18:53
@jrista
Copy link
Contributor Author

jrista commented Aug 31, 2020

It looks like our performance tests are failing. I'm a little concerned, as when I originally created those, the actual run times were well below (fractions of) the actual times I "expected". For example, the 50ms time for the 10k load was about 5x longer than the actual results I was getting at the time, which was ~10ms on average, and the 2000ms for the 1m load was about 2-3x longer. We are now topping those times consistently... I think I'd like to figure out why before we merge this. I would hate to hurt our performance numbers that much.

@jrista
Copy link
Contributor Author

jrista commented Aug 31, 2020

I'm starting to think the performance difference might in part be due to some changes in the jest libraries. I upgraded jest a minor version, and I even recall at the time that I had some issues after just with the normal run times of the full set of unit tests.

When I run the performance tests within WebStorm, they all succeed consistently, however the run times for 10k and 100k are now about half the expected times, rather than ~1/5th the expected times, and the run time of the 1m test is close to the 2s expected time.

It may be prudent to try and set up a lighter weight set of performance tests at some point...this might be testable just with node.js, as the reducer is just a function in the end.

@schuchard
Copy link
Contributor

Seem's others are seeing issues at v24 and v25 jestjs/jest#7732

@jrista
Copy link
Contributor Author

jrista commented Sep 1, 2020

Interesting...I thought I'd only upgraded a minor version, but I think I upgraded jest previously as well, and that may have been one or even two major versions in that case. Well...its not a huge issue, although it may result in tests randomly failing. When I run locally in webstorm, I seem to be around half or so the specified thresholds for those performance tests. Not as good as they were originally, but better than I'm expecting.

 + Add LoadIfNecessary action
 + Add LoadAllIfNecessary action
 + Add LoadManyIfNecessary action
 + Add LoadPageIfNecessary action
 + Add LoadRangeIfNecessary action
 * Export new actions from public api
 + Add new operators for if necessary loads
 + Add new effects for if necessary loads
 * Include new effects in standard modules
 * Export new effects from public api
 + Add stateNameOfEntity utility function
 * Update builders to use new entityStateName function

Issue #144
@jrista jrista force-pushed the feat/nea-144/loads-if-necessary branch from f7aa432 to 29f1474 Compare September 2, 2020 15:22
@jrista jrista merged commit 29f1474 into v-next Sep 2, 2020
@jrista jrista deleted the feat/nea-144/loads-if-necessary branch November 5, 2021 16:47
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.

Feature: Add new load paths to load only if data has not previously been loaded
2 participants