fix: new-sdk-draft1 #329
fix: new-sdk-draft1 #329
Conversation
aybarsayan
requested review from
ChrisChinchilla,
rflechtner and
Dudleyneedham
There was a problem hiding this comment.
I left a few small suggestions to things I know aren't quite correct, but I think @rflechtner might have more to say.
| @@ -1,16 +1,16 @@ | |||
| /* eslint-disable prefer-const */ | |||
| import * as Kilt from '@kiltprotocol/sdk-js' | |||
| import * as Did from '@kiltprotocol/did' | |||
There was a problem hiding this comment.
I am fairly certain this doesn't exist… DID-related methods are part of the Kilt.DidHelpers. and ``Kilt.DidResolver.`.
Or is this some other way to access the methods @rflechtner ?
There was a problem hiding this comment.
Unfortunately, I couldn’t find another way to access the Did.linkedInfoFromChain function. I found apiConfig.call.did.linkedInfoFromChain(encodedKiltnerd123Details), but it doesn’t seem to work the same way and gives errors.
There was a problem hiding this comment.
we are indeed missing a function to resolve a DID document based on a w3n. This could be something worth adding to the DidResolver that Chris mentioned
code_examples/sdk_examples/src/core_features/getting_started/04_fetch_endpoints.ts
Outdated
Show resolved
Hide resolved
| } = Kilt.Did.linkedInfoFromChain(encodedKiltnerd123Details) | ||
| console.log(`My name is kiltnerd123 and this is my DID: "${uri}"`) | ||
| document: { id } | ||
| } = Did.linkedInfoFromChain(encodedKiltnerd123Details) |
There was a problem hiding this comment.
This isn't an ideal way of doing things, but it's missing from the SDK, so @rflechtner is going to add a new method and then we can update this.
There was a problem hiding this comment.
Yes, as I explained above, unfortunately (as far as I saw), it’s not included in the new versions of the SDK.
| @@ -1,13 +1,12 @@ | |||
| import axios from 'axios' | |||
|
|
|||
| import * as Kilt from '@kiltprotocol/sdk-js' | |||
| import { VerifiableCredential } from '@kiltprotocol/credentials/lib/cjs/V1/types' | |||
There was a problem hiding this comment.
| import { VerifiableCredential } from '@kiltprotocol/credentials/lib/cjs/V1/types' | |
| import { types } from '@kiltprotocol/credentials' |
and then use types.VerfiableCredential. But we may want to move that to that /types package, doesn't seem right to have to access it that way
There was a problem hiding this comment.
Its fix on: 2fb4bf2
But types declaration works partially, even tho it connects the modules in types, its also says '"@kiltprotocol/credentials"' has no exported member named 'types'. Did you mean 'Types'?ts(2724)
There was a problem hiding this comment.
ah yes it may have been Types
| @@ -1,13 +1,12 @@ | |||
| import axios from 'axios' | |||
There was a problem hiding this comment.
@aybarsayan Not introduced by you, but this may be a good time to remove the axios package and replace it with a native fetch to make the HTTP requests.
There was a problem hiding this comment.
Yes, I can change this in a new pr 😊
fixes #3566
Changed the code for sdk v1, its a draft, and will continue according to feedbacks
How to test:
Please provide a brief step-by-step instruction.
If necessary provide information about dependencies (specific configuration, branches, database dumps, etc.)
Checklist: