-
Notifications
You must be signed in to change notification settings - Fork 5.5k
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
refactor: decouple node resolution from deno_core #24724
refactor: decouple node resolution from deno_core #24724
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
# Copyright 2018-2024 the Deno authors. All rights reserved. MIT license. | ||
|
||
[package] | ||
name = "node_resolver" |
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.
Make sure to claim this crate
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.
Looks like it's already claimed by us (https://crates.io/crates/node_resolver). Bump version do 0.2 then?
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.
It will be bumped to 0.2 on the next release.
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.
A bit strange that it's an ext/
if it's not implementing deno_core
extension
, but I guess not a big deal
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.
Yeah, I thought the same thing, but I didn't want to create a new top level folder 😅
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'm just going to keep it here for now. We're going to move it back to that node_resolver repo soon.
// todo(dsherret): we have the below code also in deno_core and it | ||
// would be good to somehow re-use it in both places (we don't want | ||
// to create a dependency on deno_core here) |
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.
Create a new crate for this? Not that this code changes often
No description provided.