WIP: or combinator improvement #143
Conversation
| @@ -472,7 +506,7 @@ describe('CID', () => { | |||
| const hash = await sha256.digest(textEncoder.encode('abc')) | |||
| const cid = CID.create(1, 112, hash) | |||
|
|
|||
| const parsed = CID.parse(cid.toString(base58btc)) | |||
| const parsed = /** @type {CID} */(CID.parse(cid.toString(base58btc))) | |||
There was a problem hiding this comment.
any idea why it's failing to infer this?
There was a problem hiding this comment.
It's not failing it's just return type is null|CID this manually refines it to CID, which is something TS allows.
| ], | ||
| "exclude": [ | ||
| "vendor", | ||
| "node_modules" | ||
| "node_modules", | ||
| "node_modules/multiformats" |
There was a problem hiding this comment.
this is from the ts-use test installation? or something else?
There was a problem hiding this comment.
I think this is no longer necessary, I was running into a problem where all imports were picked up from node_modules/multiformats as opposed to from src in the end mapped paths is what fixed that. This is just artifact of my trying to figure out the way.
Yeah, that is pretty silly. Although maybe the clean-up in there is worth it, it's quite nice now, aside from that. I don't mind your |
I don't know if it's not the long list of things doing |
|
I think I'll make few more changes to this pr specifically:
|
|
@Gozala : what are the next steps here? Is this work being carried forward? |
|
(question from triage) @Gozala would it be less disruptive if we update the interface instead? |
I’m not sure how would you do that without going and changing an implementation to comply with it, unless you ts-ignore it away.
Sorry I missed that comment. We can either take this as is which is better that what we have and addresses the issue at hand, or I can try to find time to make changes discussed. Those changes could be future improvements however as they would just add more functions. |
Attempt to fix #141
I like that this patch gets rid of
UnibaseDecoderall together which simplifies things a bit. However I do not like sillycomposedgetter it introduces just so that arrayreducewill not get confused by the fact that argument is eitherDecoder<B, P>orCompositeDecoder<P>.I think it would be better to either
base32.decoder.or(base64.decoder).or(base58.decoder).... Not ideal but I'm guessing number of decoders isn't going to be really big anyway.export const fromTable = (decoders) => new CompositeDecoder(decoders)so that one could simply provide table when needed as opposed to building up decoder withoroperator.P.S.: I originally left this out, because
orworks with composed and single base decoders, table on the other hand requires only single base decoders.