-
Notifications
You must be signed in to change notification settings - Fork 43
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
feat(type_tracing): store swc nodes with symbol #302
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.
LGTM, I have one question around unsafe_new()
/// WARNING: Ensure that T is a reference inside ParsedSource. Otherwise | ||
/// this is entirely unsafe. | ||
fn unsafe_new(parsed_source: &ParsedSource, value: &T) -> Self { |
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.
Maybe this should be an unsafe
fn and you can ensure T
inside it?
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 would for sure if this was public, but since it's private to the module, it's less verbose to just rely on unsafe
in the name.
What do you mean by "and you can ensure T
inside it?"?
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.
What do you mean by "and you can ensure
T
inside it?"?
I meant this part Ensure that T is a reference inside ParsedSource. Otherwise this is entirely unsafe.
. But if that's private I think it's fine.
This adds the ability to get the swc declaration from a symbol. This is useful for deno_doc because we can take a module symbol, then traverse the exports creating DocNodes.
I have good internet connection now, so I'm going to stop working on the deno_doc update after this PR and get back to other more important things.