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

Update store paths to require numbers for indices #666

Closed
wants to merge 2 commits into from

Conversation

maier49
Copy link
Contributor

@maier49 maier49 commented Feb 3, 2020

Type: feature

The following has been addressed in the PR:

Description:
This could move the store "paths" away from a JSON path style implementation and instead explicitly require numeric values to access array indices. Any string would be treated as an object property. Of course, if the targeted value is already an array passing a string would still work to index the array, but it would only use regular property access and not array methods.

This would resolve #645 but it'd also be a breaking change.

@maier49 maier49 requested review from agubler and matt-gadd February 3, 2020 23:25
@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 3, 2020

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 d17f38e:

Sandbox Source
flamboyant-moon-egw34 Configuration

@codecov
Copy link

codecov bot commented Jun 17, 2020

Codecov Report

Merging #666 into master will decrease coverage by 0.02%.
The diff coverage is 96.15%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #666      +/-   ##
==========================================
- Coverage   98.07%   98.04%   -0.03%     
==========================================
  Files         120      120              
  Lines        6634     6647      +13     
  Branches     1508     1520      +12     
==========================================
+ Hits         6506     6517      +11     
- Misses        128      130       +2     
Impacted Files Coverage Δ
src/stores/state/Patch.ts 98.46% <83.33%> (-1.54%) ⬇️
src/stores/state/Pointer.ts 98.36% <94.44%> (-1.64%) ⬇️
src/core/middleware/store.ts 100.00% <100.00%> (ø)
src/stores/Store.ts 98.90% <100.00%> (+0.06%) ⬆️
src/stores/process.ts 99.33% <100.00%> (ø)
src/stores/state/ImmutableState.ts 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 cbb3818...d17f38e. Read the comment docs.

@maier49 maier49 closed this Aug 21, 2020
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.

@dojo/framework/stores/Store should not treat strings like "123" as numbers
1 participant