Initial version with tests #1
Conversation
index.js
Outdated
| if (!converted) { | ||
| const dataView = JSON.stringify(node) | ||
| const schemaDSL = toDSL(schemaDMT) | ||
| throw new Error(`Data did not match schema\nData: ${dataView}\nSchema:${schemaDSL}`) | ||
| } |
There was a problem hiding this comment.
some fun suggestions for you to consider in addition here:
- describing the data you have as a schema so you can compare to the schema you have: https://github.com/rvagg/js-ipld-hashmap/blob/master/ipld-hashmap.js#L497-L502
- using
@ipld/printifyto print the data instead ofJSON.stringify()wich can get messy with bytes and CIDs - using dag-json to print it if you just want a compact but safe form of it
There was a problem hiding this comment.
Great call on using printify!
index.js
Outdated
|
|
||
| const schemaCID = CID.parse(schema, system.cidBases) | ||
| const schemaDMT = await system.getNode(schemaCID) | ||
| const converted = makeTyped(node, schemaDMT, type) |
There was a problem hiding this comment.
I'm not sure if there's a possibility for caching here, maybe there is since this is a multi-use "system", but if you memoize converted then it'll be even more efficient. These are generated functions that cost a little to generate up-front but should be really speedy for multiple uses. converted could be cached alongside the schema CID on the parent IPLDURLSystem.
Obviously not necessary, but if this gets reused over and over then it might be worth doing at some point.
There was a problem hiding this comment.
Hmm, Ii'd be worried about having a memory leak. Maybe if there was an LRU? What do you think about having a separate issue to track caching of schemas? It might also be relevant to caching WASM modules once we get those in.
There was a problem hiding this comment.
also not sure where the caching would take place given Schema is structured as an ADL. 🤔
index.js
Outdated
| const typedFields = new Map() | ||
| if (typeDMT.struct) { | ||
| for (const [name, fieldDMT] of Object.entries(typeDMT.struct.fields)) { | ||
| const nestedType = fieldDMT.type?.link?.expectedType |
There was a problem hiding this comment.
beware that link isn't the only possible thing in this position and that may matter
See StructField in the schema-schema, or perhaps it's easier to read now we have a typescript form of the schema-schema (!): https://github.com/ipld/js-ipld-schema/blob/fae8a4cb86f185abafddc9284bd0dec70da133c9/schema-schema.ts#L107-L123
i.e. you can also have inline defined lists and maps which can also have links in them! (foo [&Bar]). Of course you're also allowed to ignore those and say you don't support them.
There was a problem hiding this comment.
maybe also have a look at other places that inline links can show up and consider whether you need to handle them too?
There was a problem hiding this comment.
Good catch. Added some todos in the issue for this
|
pretty neat, I think this is the closest we've come to having something like go-ipld-prime's layering and integrated LinkSystem |
|
I'm gonna leave some stuff as TODOs, particularly extra tests and the migration to the ipld org. Gonna try messing around with it in js-ipfs-fetch first |
Got support for IPLD URL parsing, traversal accross links and escaped paths, support for ADLs which can asynchronously load paths, support for schemas and using schemas as sort of ADL.
TODO: