-
Notifications
You must be signed in to change notification settings - Fork 29
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
docs: Add TypeScript to getting started page #379
docs: Add TypeScript to getting started page #379
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## develop #379 +/- ##
===========================================
- Coverage 82.86% 82.13% -0.73%
===========================================
Files 19 19
Lines 1243 1282 +39
Branches 286 298 +12
===========================================
+ Hits 1030 1053 +23
- Misses 115 126 +11
- Partials 98 103 +5 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably not a big deal, but taking editing typecript for someone looking for plain javascript might not be awesome.
31dd83e
to
8620183
Compare
I would suggest actually to not go with this change in the docs and instead fix the typing at the schema levels. Ideally, the schemas should be exported from an export const LSP6Schema: ERC725JSONSchema[] = [
{
// schema here
},
// more objects
] |
But aren't the schemas usually in json files? Making them ts would be better for sure |
Co-authored-by: Hugo Masclet <[email protected]>
### JavaScript | ||
|
||
```js | ||
import { ERC725 } require('@erc725/erc725.js'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is weird
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't understand why we should duplicate the whole code snippet
just to explain that ppl can use : ERC725JSONSchema[]
I would reco to keep only one code snippet and put a comment just where needed
// Typescript type: : ERC725JSONSchema[]
or smtgh
Just to let ppl super easily spot the difference and to have less stuff to maintain
What kind of change does this PR introduce?
Update the docs
What is the current behavior?
Currently throwing errors for TS users that are unaware of built-in types.
What is the new behavior?
They instantiate their schemas with the
ERC725JSONSchema[]
type.