-
Notifications
You must be signed in to change notification settings - Fork 9
feat: Add transform dereference result hook #77
Conversation
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.
Teeeeessstttssss ;)
src/runner.ts
Outdated
const parsed = await this.parseResolveResult({ | ||
uriResult: result, | ||
// TODO: Is this correct? Result has type any, but uriResult should be type IUriResult. | ||
// uriResult: result, |
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.
Porque?
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.
uriResult
here is a just an object, but according to types.ts, it should be IUriResult
.
result comes from this line
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 undid my changes, but we should at the very least update types.ts probably.
src/runner.ts
Outdated
fragment: ref.fragment(), | ||
}); | ||
if (error) { | ||
throw new Error(`Could not transform dereferenced result for '${ref.toString()}' - ${String(error)}`); |
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.
Perhaps could set lookupResult.error
rather than throw?
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.
Let me know what you think now after moving where this get's called at.
src/types.ts
Outdated
result: any; | ||
fragment: string; | ||
uriResult: IUriResult; | ||
// uriResult: IUriResult; |
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.
Why?
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.
Answered above
🎉 This PR is included in version 2.1.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
No description provided.