Wrap rison-node to improve types #146649
Merged
Wrap rison-node to improve types #146649
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
spalger
changed the title
spalger
force-pushed
the
implement/rison-node-wrapper
branch
2 times, most recently
from
487b716
to
82a08b9
Compare
spalger
force-pushed
the
implement/rison-node-wrapper
branch
from
82a08b9
to
924c5d2
Compare
💚 Build Succeeded
Metrics [docs]Public APIs missing comments
Any counts in public APIs
Async chunks
Page load bundle
Unknown metric groupsAPI count
ESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
spalger
added
Team:Operations
|
Pinging @elastic/kibana-operations (Team:Operations) |
kqualters-elastic
approved these changes
Dec 1, 2022
botelastic
bot
added
Team:APM
|
Pinging @elastic/apm-ui (Team:APM) |
|
Pinging @elastic/fleet (Team:Fleet) |
|
Pinging @elastic/uptime (Team:uptime) |
ari-aviran
approved these changes
Dec 1, 2022
stratoula
approved these changes
Dec 1, 2022
jughosta
approved these changes
Dec 1, 2022
shahzad31
approved these changes
Dec 1, 2022
miltonhultgren
approved these changes
Dec 1, 2022
afharo
approved these changes
Dec 1, 2022
Dosant
approved these changes
Dec 1, 2022
nreese
approved these changes
Dec 1, 2022
paul-tavares
approved these changes
Dec 1, 2022
jbudz
approved these changes
Dec 1, 2022
kibanamachine
added
v8.7.0
backport:skip
1 task
maximpn
added a commit
that referenced
this pull request
Jan 5, 2023
…147218) Addresses: #140263 This PR was inspired by #146649 and while working on persistent rules table state #145111. ## Summary It includes improvements for url search parameters manipulation functionality. In particular `useGetInitialUrlParamValue` and `useReplaceUrlParams` hooks. The main idea is to isolate the encoding layer ([rison](https://github.com/Nanonid/rison)) as an implementation detail so `useGetInitialUrlParamValue` and `useReplaceUrlParams` consumers can just provide types they wish and the hooks serialize/desirealize the data into appropriate format under the hood. On top of that after `@kbn/rison` was added in #146649 it's possible to use this package directly to encode and decode parameters whenever necessary. The improvements include - store unserialized url parameters state in the redux store - encode and decode data by using functions from `@kbn/rison` directly - let `useReplaceUrlParams` accept an updater object ### Checklist - [x] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
@maximpn brought up the issues caused by the types required by the rison-node package, which attempted to communicate that "encoded values must be primitive values, or recursive arrays/object of primitive values". This isn't actually expressible in TypeScript, which lead to many instances of
rison.encode(value as unknown as RisonValue)which is useless. Additionally, the rison-node library actually supports any value and will either produce valid rison orundefinedfor that value.To address this I'm adding a wrapper function which accepts
anyand returns astring. If rison-node is totally unable to produce any rison for the value (because the value isundefinedor some other type like Symbol or BigInt) theencode()function will throw. If you're accepting arbitrary input you can use theencodeUnknown()function, which will return a string or undefined, if the value you provided has zero rison representation.Like JSON.stringify() any non-circular primitive, object, or array can be encoded with either function. If the values within those objects are not encodable (functions, RegExps, etc) then they will be skipped. Any object/array with the
toJSON()method will be converted to JSON first, and if the prototype of the object has theencode_rison()method it will be used to convert he value into rison.The changes in this PR are mostly updating usage of rison-node to use
@kbn/rison(which is also enforced by eslint). There are also several changes which remove unnecessary casting.