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

export all relevant types #106

Closed
wants to merge 2 commits into from
Closed

Conversation

pax-k
Copy link

@pax-k pax-k commented Jun 2, 2023

Fixes:

The inferred type of 'createProtocolRecord' cannot be named without a reference to '../../../node_modules/@tbd54566975/web5/dist/types/record.js'. This is likely not portable. A type annotation is necessary.ts(2742)

What we are doing:

import type { Web5 } from '@tbd54566975/web5'

async createProtocolRecord() {
...
  const response = await this.web5.dwn.records.create({
        data: recordData,
        message: message,
      })
  return response.record!
}

@pax-k pax-k changed the title export record type export all relevant types Jun 2, 2023
@codecov
Copy link

codecov bot commented Jun 6, 2023

Codecov Report

Merging #106 (8bffe5b) into main (2ac756f) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #106   +/-   ##
=======================================
  Coverage   65.64%   65.64%           
=======================================
  Files          31       31           
  Lines        3397     3397           
  Branches      195      195           
=======================================
  Hits         2230     2230           
  Misses       1165     1165           
  Partials        2        2           
Components Coverage Δ
crypto 0.00% <ø> (ø)
dids 48.61% <ø> (ø)
web5 81.18% <ø> (ø)
web5-agent 0.00% <ø> (ø)
web5-proxy-agent 0.00% <ø> (ø)
web5-user-agent 79.34% <ø> (ø)

@michaelneale
Copy link
Contributor

didn't break anything - only risk is that this may be exposing more of an api that originally (of course) but some of those things may be more likely to change (possibly) - just an FYI. What is the normal typescript thing to do - have external types contained in one file?

@pax-k
Copy link
Author

pax-k commented Jun 6, 2023

Hey! The reason for exporting types is that, for example, I'm writing a wrapper on top of web5-js. So far, I only needed ProtocolDefinition and RecordsWriteRequest, and with my PR I thought to export all types, just to be sure a future PR won't be needed. What do you think?

@michaelneale
Copy link
Contributor

ok with me and makes sense - @mistermoe @frankhinek any thoughts on this?

@frankhinek frankhinek mentioned this pull request Aug 21, 2023
@frankhinek
Copy link
Contributor

Addressed in PR #168

@frankhinek frankhinek closed this Aug 21, 2023
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.

3 participants