-
-
Notifications
You must be signed in to change notification settings - Fork 714
feat(wasm): expose Workspace::scan_project_folder #7005
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
Conversation
|
Name | Type |
---|---|
@biomejs/biome | Minor |
@biomejs/wasm-bundler | Minor |
@biomejs/wasm-nodejs | Minor |
@biomejs/wasm-web | Minor |
@biomejs/js-api | Major |
@biomejs/cli-win32-x64 | Minor |
@biomejs/cli-win32-arm64 | Minor |
@biomejs/cli-darwin-x64 | Minor |
@biomejs/cli-darwin-arm64 | Minor |
@biomejs/cli-linux-x64 | Minor |
@biomejs/cli-linux-arm64 | Minor |
@biomejs/cli-linux-x64-musl | Minor |
@biomejs/cli-linux-arm64-musl | Minor |
@biomejs/backend-jsonrpc | Patch |
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
#[instrument(level = "debug", skip(ctx))] | ||
fn scan_folder(folder: &Utf8Path, ctx: ScanContext) -> ScanFolderResult { | ||
let start = Instant::now(); | ||
let start = web_time::Instant::now(); |
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.
web_time::Instant
is a drop-in replacement of std::time::Instant
backed by performance.now()
with wasm-bindgen.
CodSpeed Performance ReportMerging #7005 will not alter performanceComparing Summary
|
9cfe24f
to
bf11e53
Compare
.changeset/silly-colts-dress.md
Outdated
@@ -0,0 +1,5 @@ | |||
--- | |||
"@biomejs/biome": patch |
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.
Since this change primarily affects the wasm packages, probably should also include the wasm packages in this list.
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.
@dyc3 Actually we only use @biomejs/biome
and @biomejs/js-api
for changing which packages to ship. Bumping @biomejs/biome
also ships WASM packages.
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.
Right, but when the changelogs get rendered out, the change only shows up for a package if its listed in the changeset. Each package has it's own changelog.
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 agree, WASM API changes should be listed in the changelogs for @biome/wasm-*
. However, we don't render changelogs for packages other than @biome/biome
and @biome/js-api
for now, so adding WASM packages is meaningless. Let me align it with #6896 for this time.
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 think @dyc3 makes a good point. We usually publish everything together, but we can add a changelog line for those packages. I can update the changeset of the other PR (as long as they all are patch/minor).
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 merged the changeset to yours, adding wasm-*
packages to the list. be2d768
.changeset/silly-colts-dress.md
Outdated
@@ -0,0 +1,5 @@ | |||
--- | |||
"@biomejs/biome": patch |
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 did the same here #6896 but merged into next
. I think it's fine to have them shipped in patches, but we should agree how we should ship new APIs from core :)
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.
Didn't notice your PR 😔 Then should I close this as I suppose exposing update_module_graph is sufficient to enable project rules?
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 exposed those functions because I thought they were more useful for the playground, where there are single files.
The methods I exposed should be useful for project rules, however they require more plumbing.
This doesn't mean that this new method is useless, because we could use it for the JavaScript APIs as well :)
Let's merge the PR nonetheless :)
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'll rebase this to next
and ship with the methods you added :)
bf11e53
to
19713e6
Compare
19713e6
to
38de486
Compare
38de486
to
be2d768
Compare
Summary
Exposes a workspace method
scan_project_folder
via the WASM API. This will enable use of type inference and project rules in the playground.Test Plan
Manually tested with the playground locally.
Docs
N/A