Conversation
✅ Deploy Preview for biomejs ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Lunaria Status Overview🌕 This pull request will trigger status changes. Learn moreBy default, every PR changing files present in the Lunaria configuration's You can change this by adding one of the keywords present in the Tracked Files
Warnings reference
|
WalkthroughDocumentation updates only. Added a new "Globs resolution" explanatory section to the big-projects guide describing how globs in Suggested labels
Pre-merge checks and finishing touches✅ Passed checks (4 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/content/docs/guides/big-projects.mdxsrc/content/docs/reference/configuration.mdx
🧰 Additional context used
🪛 LanguageTool
src/content/docs/guides/big-projects.mdx
[grammar] ~154-~154: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...llowing project is placed in the folder /Users/todo, and contains a root configuration in ...
(TO_DO_HYPHEN)
[grammar] ~154-~154: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ..., and contains a root configuration in /Users/todo/biome.json, and one inside /Users/tod...
(TO_DO_HYPHEN)
[grammar] ~155-~155: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: .../Users/todo/biome.json, and one inside /Users/todo/app/biome.json. - Globs defined in /...
(TO_DO_HYPHEN)
[grammar] ~157-~157: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...do/app/biome.json. - Globs defined in /Users/todo/biome.jsonare resolved from/Users/t...
(TO_DO_HYPHEN)
[grammar] ~157-~157: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...sers/todo/biome.jsonare resolved from/Users/todo/. - Globs defined in /Users/todo/app/...
(TO_DO_HYPHEN)
[grammar] ~158-~158: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...from /Users/todo/. - Globs defined in /Users/todo/app/biome.json are resolved from `/Use...
(TO_DO_HYPHEN)
[grammar] ~158-~158: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: .../todo/app/biome.jsonare resolved from/Users/todo/app`. The root configuration excludes ...
(TO_DO_HYPHEN)
[grammar] ~188-~188: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...f/when you decide to run a command from /Users/todo/app**. That's because when Biome resol...
(TO_DO_HYPHEN)
[grammar] ~189-~189: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...pp/generated.tsis resolved like this:/Users/todo/app ~ !app/generatd.ts`, which doesn't ...
(TO_DO_HYPHEN)
[typographical] ~192-~192: The conjunction “so that” does not require a comma.
Context: ...wo options: - Prepend the glob with **, so that any folder is matched: ```json title=...
(SO_THAT_UNNECESSARY_COMMA)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Redirect rules - biomejs
- GitHub Check: Header rules - biomejs
- GitHub Check: Pages changed - biomejs
- GitHub Check: test
🔇 Additional comments (1)
src/content/docs/reference/configuration.mdx (1)
60-60: LGTM! Clear and consistent clarification.The added sentence "All globs are resolved from the configuration file where they are defined." is consistently applied across all relevant sections and directly addresses the user confusion mentioned in issue #8518. Well done!
Also applies to: 250-250, 399-399, 513-513, 1494-1494
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/content/docs/guides/big-projects.mdx (2)
152-152: Clarify the field reference.The term
*.includesis a bit ambiguous—it could be read as a glob pattern rather than referring to configuration fields. Consider being more explicit, such as "fields ending with.includes" or listing them (e.g., "files.includes,linter.includes, etc.").
188-189: Consider clarifying the tilde notation.The expression
/Users/todo/app ~ !app/generated.tsuses a tilde to illustrate glob resolution, but this notation isn't standard and might confuse readers. Consider rephrasing to something like "resolved as/Users/todo/app/app/generated.ts" or "interpreted relative to/Users/todo/app" for clarity.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/content/docs/guides/big-projects.mdx
🧰 Additional context used
🪛 LanguageTool
src/content/docs/guides/big-projects.mdx
[grammar] ~154-~154: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...llowing project is placed in the folder /Users/todo, and contains a root configuration in ...
(TO_DO_HYPHEN)
[grammar] ~154-~154: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ..., and contains a root configuration in /Users/todo/biome.json, and one inside /Users/tod...
(TO_DO_HYPHEN)
[grammar] ~155-~155: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: .../Users/todo/biome.json, and one inside /Users/todo/app/biome.json. - Globs defined in /...
(TO_DO_HYPHEN)
[grammar] ~157-~157: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...do/app/biome.json. - Globs defined in /Users/todo/biome.jsonare resolved from/Users/t...
(TO_DO_HYPHEN)
[grammar] ~157-~157: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...sers/todo/biome.jsonare resolved from/Users/todo/. - Globs defined in /Users/todo/app/...
(TO_DO_HYPHEN)
[grammar] ~158-~158: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...from /Users/todo/. - Globs defined in /Users/todo/app/biome.json are resolved from `/Use...
(TO_DO_HYPHEN)
[grammar] ~158-~158: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: .../todo/app/biome.jsonare resolved from/Users/todo/app`. The root configuration excludes ...
(TO_DO_HYPHEN)
[grammar] ~188-~188: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...f/when you decide to run a command from /Users/todo/app**. That's because when Biome resol...
(TO_DO_HYPHEN)
[grammar] ~189-~189: It appears that a hyphen is missing in the noun “to-do” (= task) or did you mean the verb “to do”?
Context: ...pp/generated.tsis resolved like this:/Users/todo/app ~ !app/generated.ts`, which doesn't...
(TO_DO_HYPHEN)
[typographical] ~192-~192: The conjunction “so that” does not require a comma.
Context: ...wo options: - Prepend the glob with **, so that any folder is matched: ```json title=...
(SO_THAT_UNNECESSARY_COMMA)
🔇 Additional comments (1)
src/content/docs/guides/big-projects.mdx (1)
150-204: Nice addition—addresses the confusion well.This section directly tackles the issue from #8518 and provides concrete examples with clear solutions. The file tree and configuration snippets make the problem and fix easy to grasp.
|
|
||
| A list of [glob patterns](#glob-syntax-reference) of files to process. | ||
|
|
||
| All globs are resolved from the configuration file where they are defined. |
There was a problem hiding this comment.
@ematipico this seems opposite of the above-- they are not resolved from the config file they are defined, but rather from the "leaf" or "package" config file, right?
There was a problem hiding this comment.
Fair. What about "configuration where they are loaded from"?
| inside `/Users/todo/app/biome.json`. | ||
|
|
||
| - Globs defined in `/Users/todo/biome.json` are resolved from `/Users/todo/`. | ||
| - Globs defined in `/Users/todo/app/biome.json` are resolved from `/Users/todo/app`. |
There was a problem hiding this comment.
I think part of the confusion may arise from this kind of description. From my perspective, these two points contradict the actual behavior, because the root glob is evaled from the config file it was defined in only if no other config file inherits from it. If a package config file inherits from the root, then globs are not evaled from the file they are defined in but rather the file that inherits them.
There was a problem hiding this comment.
I agree it's confusing if the docs and the behaviour are contradicting one another. But I do think the behaviour as described here sounds the most sensible, so I would consider the actual behaviour to be a bug then.
| the extended glob `!app/generated.ts` is resolved like this: `/Users/todo/app ~ !app/generated.ts`, which doesn't match any existing folder inside the project. | ||
|
|
||
| To properly exclude `app/generated.ts`, you have two options: | ||
| - Prepend the glob with `**`, so that any folder is matched: |
There was a problem hiding this comment.
This doesn't look right-- prepending ** doesn't help because the /app/ part will still not match anything because are are executing from the app directory.
You would need to remove the package path from the root config so that it sees the package relative path correctly.
There was a problem hiding this comment.
I'm not sure I understand. The objective is to exclude app/generated.ts, not generated.ts.
What am I missing? Please provide an example so I can understand
There was a problem hiding this comment.
If you have the paths:
biome.json
app/generated.ts
app/biome.json
and app/biome.json extends from biome.json, then including !**/app/generated.ts in the root means it is ingested into the app config as-is. That means from the app conffig, the glob resolves as !**/app/generated.ts. The glob prefix checks the current dir and any subdirs for app/ and finds nothing, because the app dir is what has already been traversed in the transform from root->app config. It doesn't do anything because you aren't adding an unknown prefix in the transition from root->app. You are removing a known prefix.
Prefixing a ** to the glob only helps if there are no intermediate directories between the root and the package configs. If there are, then they likely interfere and render the glob useless.
Let me know if I am making any sense 🤞
| } | ||
| } | ||
| ``` | ||
| - Move the `!app/generated.ts` glob inside `/Users/todo/app/biome.json` |
There was a problem hiding this comment.
How is this done? Is there a way to manipulate the global files.includes from configs that inherit the root? The json schema nor the config file reference doesn't seem to think so.
It may be worth clarifying what exactly you can do if you want to move the path into the package config. I think something like:
"formatter": {
"enabled": false
},
"linter": {
"enabled": false
},
"assist": {
"enabled": false
}
in an exclude, but I am not 100% on that.
3 participants
Summary
Closes biomejs/biome#8518
It seems some users were confused by the behaviour of
files.includes, and it was pointed out that there was a gap in our docs.This PR tries to fill that gap.
@brandonw please let me know if the additions are clear enough for you