Skip to content
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

fix: only watch lock files in workspace root #91

Merged
merged 6 commits into from
Jan 24, 2024
Merged

Conversation

ChiefORZ
Copy link
Contributor

@ChiefORZ ChiefORZ commented Jan 22, 2024

src/main.ts Outdated Show resolved Hide resolved
@ChiefORZ ChiefORZ requested a review from ematipico January 22, 2024 07:58
@nhedger
Copy link
Member

nhedger commented Jan 22, 2024

I'm wondering if instead we shouldn't just watch lockfiles in the root of the workspace folder.

@ChiefORZ
Copy link
Contributor Author

@nhedger but then there could be the risk that lockfiles in a multi-root vscode workspace - will not trigger a reload. just a mind of though because i personally work with multi-root workspaces. but im no expert in how vscode works

@nhedger
Copy link
Member

nhedger commented Jan 23, 2024

At this time we do not support multiple workspace folders. We're hoping to add support soon though.

That being said, nothing would prevent us from creating a watcher per workspace folder if we needed to.

The reason I proposed this is because currently the extension will use biome from the dependencies declared at the root of your project anyway, so watching lock files deeper in the hierarchy is basically useless.

@nhedger
Copy link
Member

nhedger commented Jan 23, 2024

I've tested the code, and unfortunately, it looks like VS Code's workspace.createFileSystemWatcher function does not yet support excluding files from watching, so this glob pattern won't prevent changes in .wireit/lock.lock files from being detected.

There is an open issue about it here: microsoft/vscode#169724

In the meantime, I think changing the glob pattern to only watch for lock files in the root of the workspace folder is the way to go.

@ChiefORZ
Copy link
Contributor Author

@nhedger the PR always fails because in our codebase the "source.organizeImports.biome" does not work like it is described in biome's docs - we need the additional "source.organizeImports" to be enabled too.

{
  "editor.codeActionsOnSave": {
    "source.organizeImports": "explicit",
    "source.organizeImports.biome": "explicit",
  }
}

With this codebase there is a race condition, so i have to disable the "source.organizeImports" rule and it works. But then our codebase stops working - do you maybe have any insights in why this is happening?

@nhedger
Copy link
Member

nhedger commented Jan 24, 2024

Gotcha, would you mind filing an issue for this problem?

// Best way to determine package updates. Will work for npm, yarn, pnpm and bun. (Might work for more files also).
// It is not possible to listen node_modules, because it is usually gitignored.
const watcher = workspace.createFileSystemWatcher(
new RelativePattern(workspace.workspaceFolders[0], "*lock*"),
Copy link
Member

@nhedger nhedger Jan 24, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated to a RelativePattern, because otherwise the watcher won't work as expected. See microsoft/vscode#31602

@nhedger
Copy link
Member

nhedger commented Jan 24, 2024

Thanks for you contribution @ChiefORZ!

@nhedger nhedger changed the title fix: ignore .wireit folder for determine package updates fix: only watch lock files in workspace root Jan 24, 2024
@nhedger nhedger merged commit cd6ef47 into biomejs:main Jan 24, 2024
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

🐛 Incompatibility with wireit
3 participants