Add --error-on-warnings to hooks - #59

Open
tyilo wants to merge 1 commit into
biomejs:mainfrom
tyilo:fix-hooks
Open

Add --error-on-warnings to hooks#59
tyilo wants to merge 1 commit into
biomejs:mainfrom
tyilo:fix-hooks

Conversation

@tyilo

Copy link
Copy Markdown
Contributor

Fixes#58

@ematipico

Copy link
Copy Markdown
Member

I don't think this is a good idea. There might be users that want and accepts warnings in their code.

@tyilo

tyilo commented Aug 20, 2025

Copy link
Copy Markdown
ContributorAuthor

I don't think this is a good idea. There might be users that want and accepts warnings in their code.

I don't really see the use-case for running biome lint with pre-commit without this flag. pre-commit won't even show the warnings with a 0 exit code, so I think this makes the hook useless.

Furthermore, the default pre-commit handlers for other linters defaults to failing when there is an unfixed lint.

If the user really wants to have warnings in their code then I don't think pre-commit is the right tool, as the warnings won't even be shown. However, if there is some weird use-case, they can override the entrypoint in their pre-commit config.

@ematipico

ematipico commented Aug 22, 2025

Copy link
Copy Markdown
Member

Still, Biome emits warnings that sometimes fall outside the linter's ecosphere, meaning users can't control them directly until they are fixed. This means that every time we decide to emit a warning, this will fail. For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

As you said, users can customise the entrypoint, so for now you can do that too, until we (the team) decides how we should handle our internal diagnostics.

@tyilo

Copy link
Copy Markdown
ContributorAuthor

Still, Biome emits warnings that sometimes fall outside the linter's ecosphere, meaning users can't control them directly until they are fixed. This means that every time we decide to emit a warning, this will fail. For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

Fair enough. Is there another flag for only exiting with a non-zero exit code when there are lints?
Or maybe biome should by default have a non-zero exit code, when there are lints.

@matthiask

Copy link
Copy Markdown

I always run the pre-commit hook with verbose:true so that I see the warnings even when I don't have any errors. It might make sense to recommend verbose:true -- the warnings are useful but I'd suffer very much if all warnings were errors :-)

@Guiorgy

Guiorgy commented Mar 1, 2026

Copy link
Copy Markdown

[...] For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

That's a surprising issue 😅.

As you said, users can customise the entrypoint, so for now you can do that too, until we (the team) decides how we should handle our internal diagnostics.

Overwriting the entrypoint is an overkill, we can append additional args:

- repo: https://github.com/biomejs/pre-commitrev: v2.4.4hooks:
- id: biome-checkargs: ["--error-on-warnings"]

From pre-commit docs:

args: (optional) list of additional parameters to pass to the hook.

Sign up for freeto 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.

biome-check hook doesn't fail when there are lints

4 participants

@tyilo@ematipico@matthiask@Guiorgy
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

Add --error-on-warnings to hooks - #59

Open
tyilo wants to merge 1 commit into
biomejs:mainfrom
tyilo:fix-hooks
Open

Add --error-on-warnings to hooks#59
tyilo wants to merge 1 commit into
biomejs:mainfrom
tyilo:fix-hooks

Conversation

@tyilo

Copy link
Copy Markdown
Contributor

Fixes#58

@ematipico

Copy link
Copy Markdown
Member

I don't think this is a good idea. There might be users that want and accepts warnings in their code.

@tyilo

tyilo commented Aug 20, 2025

Copy link
Copy Markdown
ContributorAuthor

I don't think this is a good idea. There might be users that want and accepts warnings in their code.

I don't really see the use-case for running biome lint with pre-commit without this flag. pre-commit won't even show the warnings with a 0 exit code, so I think this makes the hook useless.

Furthermore, the default pre-commit handlers for other linters defaults to failing when there is an unfixed lint.

If the user really wants to have warnings in their code then I don't think pre-commit is the right tool, as the warnings won't even be shown. However, if there is some weird use-case, they can override the entrypoint in their pre-commit config.

@ematipico

ematipico commented Aug 22, 2025

Copy link
Copy Markdown
Member

Still, Biome emits warnings that sometimes fall outside the linter's ecosphere, meaning users can't control them directly until they are fixed. This means that every time we decide to emit a warning, this will fail. For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

As you said, users can customise the entrypoint, so for now you can do that too, until we (the team) decides how we should handle our internal diagnostics.

@tyilo

Copy link
Copy Markdown
ContributorAuthor

Still, Biome emits warnings that sometimes fall outside the linter's ecosphere, meaning users can't control them directly until they are fixed. This means that every time we decide to emit a warning, this will fail. For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

Fair enough. Is there another flag for only exiting with a non-zero exit code when there are lints?
Or maybe biome should by default have a non-zero exit code, when there are lints.

@matthiask

Copy link
Copy Markdown

I always run the pre-commit hook with verbose:true so that I see the warnings even when I don't have any errors. It might make sense to recommend verbose:true -- the warnings are useful but I'd suffer very much if all warnings were errors :-)

@Guiorgy

Guiorgy commented Mar 1, 2026

Copy link
Copy Markdown

[...] For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

That's a surprising issue 😅.

As you said, users can customise the entrypoint, so for now you can do that too, until we (the team) decides how we should handle our internal diagnostics.

Overwriting the entrypoint is an overkill, we can append additional args:

- repo: https://github.com/biomejs/pre-commitrev: v2.4.4hooks:
- id: biome-checkargs: ["--error-on-warnings"]

From pre-commit docs:

args: (optional) list of additional parameters to pass to the hook.

Sign up for freeto 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.

biome-check hook doesn't fail when there are lints

4 participants

@tyilo@ematipico@matthiask@Guiorgy
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add --error-on-warnings to hooks - #59

Open
tyilo wants to merge 1 commit into
biomejs:mainfrom
tyilo:fix-hooks
Open

Add --error-on-warnings to hooks#59
tyilo wants to merge 1 commit into
biomejs:mainfrom
tyilo:fix-hooks

Conversation

@tyilo

Copy link
Copy Markdown
Contributor

Fixes#58

@ematipico

Copy link
Copy Markdown
Member

I don't think this is a good idea. There might be users that want and accepts warnings in their code.

@tyilo

tyilo commented Aug 20, 2025

Copy link
Copy Markdown
ContributorAuthor

I don't think this is a good idea. There might be users that want and accepts warnings in their code.

I don't really see the use-case for running biome lint with pre-commit without this flag. pre-commit won't even show the warnings with a 0 exit code, so I think this makes the hook useless.

Furthermore, the default pre-commit handlers for other linters defaults to failing when there is an unfixed lint.

If the user really wants to have warnings in their code then I don't think pre-commit is the right tool, as the warnings won't even be shown. However, if there is some weird use-case, they can override the entrypoint in their pre-commit config.

@ematipico

ematipico commented Aug 22, 2025

Copy link
Copy Markdown
Member

Still, Biome emits warnings that sometimes fall outside the linter's ecosphere, meaning users can't control them directly until they are fixed. This means that every time we decide to emit a warning, this will fail. For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

As you said, users can customise the entrypoint, so for now you can do that too, until we (the team) decides how we should handle our internal diagnostics.

@tyilo

Copy link
Copy Markdown
ContributorAuthor

Still, Biome emits warnings that sometimes fall outside the linter's ecosphere, meaning users can't control them directly until they are fixed. This means that every time we decide to emit a warning, this will fail. For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

Fair enough. Is there another flag for only exiting with a non-zero exit code when there are lints?
Or maybe biome should by default have a non-zero exit code, when there are lints.

@matthiask

Copy link
Copy Markdown

I always run the pre-commit hook with verbose:true so that I see the warnings even when I don't have any errors. It might make sense to recommend verbose:true -- the warnings are useful but I'd suffer very much if all warnings were errors :-)

@Guiorgy

Guiorgy commented Mar 1, 2026

Copy link
Copy Markdown

[...] For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

That's a surprising issue 😅.

As you said, users can customise the entrypoint, so for now you can do that too, until we (the team) decides how we should handle our internal diagnostics.

Overwriting the entrypoint is an overkill, we can append additional args:

- repo: https://github.com/biomejs/pre-commitrev: v2.4.4hooks:
- id: biome-checkargs: ["--error-on-warnings"]

From pre-commit docs:

args: (optional) list of additional parameters to pass to the hook.

Sign up for freeto 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.

biome-check hook doesn't fail when there are lints

4 participants

@tyilo@ematipico@matthiask@Guiorgy
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add --error-on-warnings to hooks - #59

Open
tyilo wants to merge 1 commit into
biomejs:mainfrom
tyilo:fix-hooks
Open

Add --error-on-warnings to hooks#59
tyilo wants to merge 1 commit into
biomejs:mainfrom
tyilo:fix-hooks

Conversation

@tyilo

Copy link
Copy Markdown
Contributor

Fixes#58

@ematipico

Copy link
Copy Markdown
Member

I don't think this is a good idea. There might be users that want and accepts warnings in their code.

@tyilo

tyilo commented Aug 20, 2025

Copy link
Copy Markdown
ContributorAuthor

I don't think this is a good idea. There might be users that want and accepts warnings in their code.

I don't really see the use-case for running biome lint with pre-commit without this flag. pre-commit won't even show the warnings with a 0 exit code, so I think this makes the hook useless.

Furthermore, the default pre-commit handlers for other linters defaults to failing when there is an unfixed lint.

If the user really wants to have warnings in their code then I don't think pre-commit is the right tool, as the warnings won't even be shown. However, if there is some weird use-case, they can override the entrypoint in their pre-commit config.

@ematipico

ematipico commented Aug 22, 2025

Copy link
Copy Markdown
Member

Still, Biome emits warnings that sometimes fall outside the linter's ecosphere, meaning users can't control them directly until they are fixed. This means that every time we decide to emit a warning, this will fail. For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

As you said, users can customise the entrypoint, so for now you can do that too, until we (the team) decides how we should handle our internal diagnostics.

@tyilo

Copy link
Copy Markdown
ContributorAuthor

Still, Biome emits warnings that sometimes fall outside the linter's ecosphere, meaning users can't control them directly until they are fixed. This means that every time we decide to emit a warning, this will fail. For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

Fair enough. Is there another flag for only exiting with a non-zero exit code when there are lints?
Or maybe biome should by default have a non-zero exit code, when there are lints.

@matthiask

Copy link
Copy Markdown

I always run the pre-commit hook with verbose:true so that I see the warnings even when I don't have any errors. It might make sense to recommend verbose:true -- the warnings are useful but I'd suffer very much if all warnings were errors :-)

@Guiorgy

Guiorgy commented Mar 1, 2026

Copy link
Copy Markdown

[...] For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

That's a surprising issue 😅.

As you said, users can customise the entrypoint, so for now you can do that too, until we (the team) decides how we should handle our internal diagnostics.

Overwriting the entrypoint is an overkill, we can append additional args:

- repo: https://github.com/biomejs/pre-commitrev: v2.4.4hooks:
- id: biome-checkargs: ["--error-on-warnings"]

From pre-commit docs:

args: (optional) list of additional parameters to pass to the hook.

Sign up for freeto 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.

biome-check hook doesn't fail when there are lints

4 participants

@tyilo@ematipico@matthiask@Guiorgy
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

Add --error-on-warnings to hooks - #59

Open
tyilo wants to merge 1 commit into
biomejs:mainfrom
tyilo:fix-hooks
Open

Add --error-on-warnings to hooks#59
tyilo wants to merge 1 commit into
biomejs:mainfrom
tyilo:fix-hooks

Conversation

@tyilo

Copy link
Copy Markdown
Contributor

Fixes#58

@ematipico

Copy link
Copy Markdown
Member

I don't think this is a good idea. There might be users that want and accepts warnings in their code.

@tyilo

tyilo commented Aug 20, 2025

Copy link
Copy Markdown
ContributorAuthor

I don't think this is a good idea. There might be users that want and accepts warnings in their code.

I don't really see the use-case for running biome lint with pre-commit without this flag. pre-commit won't even show the warnings with a 0 exit code, so I think this makes the hook useless.

Furthermore, the default pre-commit handlers for other linters defaults to failing when there is an unfixed lint.

If the user really wants to have warnings in their code then I don't think pre-commit is the right tool, as the warnings won't even be shown. However, if there is some weird use-case, they can override the entrypoint in their pre-commit config.

@ematipico

ematipico commented Aug 22, 2025

Copy link
Copy Markdown
Member

Still, Biome emits warnings that sometimes fall outside the linter's ecosphere, meaning users can't control them directly until they are fixed. This means that every time we decide to emit a warning, this will fail. For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

As you said, users can customise the entrypoint, so for now you can do that too, until we (the team) decides how we should handle our internal diagnostics.

@tyilo

Copy link
Copy Markdown
ContributorAuthor

Still, Biome emits warnings that sometimes fall outside the linter's ecosphere, meaning users can't control them directly until they are fixed. This means that every time we decide to emit a warning, this will fail. For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

Fair enough. Is there another flag for only exiting with a non-zero exit code when there are lints?
Or maybe biome should by default have a non-zero exit code, when there are lints.

@matthiask

Copy link
Copy Markdown

I always run the pre-commit hook with verbose:true so that I see the warnings even when I don't have any errors. It might make sense to recommend verbose:true -- the warnings are useful but I'd suffer very much if all warnings were errors :-)

@Guiorgy

Guiorgy commented Mar 1, 2026

Copy link
Copy Markdown

[...] For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

That's a surprising issue 😅.

As you said, users can customise the entrypoint, so for now you can do that too, until we (the team) decides how we should handle our internal diagnostics.

Overwriting the entrypoint is an overkill, we can append additional args:

- repo: https://github.com/biomejs/pre-commitrev: v2.4.4hooks:
- id: biome-checkargs: ["--error-on-warnings"]

From pre-commit docs:

args: (optional) list of additional parameters to pass to the hook.

Sign up for freeto 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.

biome-check hook doesn't fail when there are lints

4 participants

@tyilo@ematipico@matthiask@Guiorgy
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add --error-on-warnings to hooks - #59

Open
tyilo wants to merge 1 commit into
biomejs:mainfrom
tyilo:fix-hooks
Open

Add --error-on-warnings to hooks#59
tyilo wants to merge 1 commit into
biomejs:mainfrom
tyilo:fix-hooks

Conversation

@tyilo

Copy link
Copy Markdown
Contributor

Fixes#58

@ematipico

Copy link
Copy Markdown
Member

I don't think this is a good idea. There might be users that want and accepts warnings in their code.

@tyilo

tyilo commented Aug 20, 2025

Copy link
Copy Markdown
ContributorAuthor

I don't think this is a good idea. There might be users that want and accepts warnings in their code.

I don't really see the use-case for running biome lint with pre-commit without this flag. pre-commit won't even show the warnings with a 0 exit code, so I think this makes the hook useless.

Furthermore, the default pre-commit handlers for other linters defaults to failing when there is an unfixed lint.

If the user really wants to have warnings in their code then I don't think pre-commit is the right tool, as the warnings won't even be shown. However, if there is some weird use-case, they can override the entrypoint in their pre-commit config.

@ematipico

ematipico commented Aug 22, 2025

Copy link
Copy Markdown
Member

Still, Biome emits warnings that sometimes fall outside the linter's ecosphere, meaning users can't control them directly until they are fixed. This means that every time we decide to emit a warning, this will fail. For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

As you said, users can customise the entrypoint, so for now you can do that too, until we (the team) decides how we should handle our internal diagnostics.

@tyilo

Copy link
Copy Markdown
ContributorAuthor

Still, Biome emits warnings that sometimes fall outside the linter's ecosphere, meaning users can't control them directly until they are fixed. This means that every time we decide to emit a warning, this will fail. For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

Fair enough. Is there another flag for only exiting with a non-zero exit code when there are lints?
Or maybe biome should by default have a non-zero exit code, when there are lints.

@matthiask

Copy link
Copy Markdown

I always run the pre-commit hook with verbose:true so that I see the warnings even when I don't have any errors. It might make sense to recommend verbose:true -- the warnings are useful but I'd suffer very much if all warnings were errors :-)

@Guiorgy

Guiorgy commented Mar 1, 2026

Copy link
Copy Markdown

[...] For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

That's a surprising issue 😅.

As you said, users can customise the entrypoint, so for now you can do that too, until we (the team) decides how we should handle our internal diagnostics.

Overwriting the entrypoint is an overkill, we can append additional args:

- repo: https://github.com/biomejs/pre-commitrev: v2.4.4hooks:
- id: biome-checkargs: ["--error-on-warnings"]

From pre-commit docs:

args: (optional) list of additional parameters to pass to the hook.

Sign up for freeto 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.

biome-check hook doesn't fail when there are lints

4 participants

@tyilo@ematipico@matthiask@Guiorgy
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

Add --error-on-warnings to hooks - #59

Open
tyilo wants to merge 1 commit into
biomejs:mainfrom
tyilo:fix-hooks
Open

Add --error-on-warnings to hooks#59
tyilo wants to merge 1 commit into
biomejs:mainfrom
tyilo:fix-hooks

Conversation

@tyilo

Copy link
Copy Markdown
Contributor

Fixes#58

@ematipico

Copy link
Copy Markdown
Member

I don't think this is a good idea. There might be users that want and accepts warnings in their code.

@tyilo

tyilo commented Aug 20, 2025

Copy link
Copy Markdown
ContributorAuthor

I don't think this is a good idea. There might be users that want and accepts warnings in their code.

I don't really see the use-case for running biome lint with pre-commit without this flag. pre-commit won't even show the warnings with a 0 exit code, so I think this makes the hook useless.

Furthermore, the default pre-commit handlers for other linters defaults to failing when there is an unfixed lint.

If the user really wants to have warnings in their code then I don't think pre-commit is the right tool, as the warnings won't even be shown. However, if there is some weird use-case, they can override the entrypoint in their pre-commit config.

@ematipico

ematipico commented Aug 22, 2025

Copy link
Copy Markdown
Member

Still, Biome emits warnings that sometimes fall outside the linter's ecosphere, meaning users can't control them directly until they are fixed. This means that every time we decide to emit a warning, this will fail. For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

As you said, users can customise the entrypoint, so for now you can do that too, until we (the team) decides how we should handle our internal diagnostics.

@tyilo

Copy link
Copy Markdown
ContributorAuthor

Still, Biome emits warnings that sometimes fall outside the linter's ecosphere, meaning users can't control them directly until they are fixed. This means that every time we decide to emit a warning, this will fail. For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

Fair enough. Is there another flag for only exiting with a non-zero exit code when there are lints?
Or maybe biome should by default have a non-zero exit code, when there are lints.

@matthiask

Copy link
Copy Markdown

I always run the pre-commit hook with verbose:true so that I see the warnings even when I don't have any errors. It might make sense to recommend verbose:true -- the warnings are useful but I'd suffer very much if all warnings were errors :-)

@Guiorgy

Guiorgy commented Mar 1, 2026

Copy link
Copy Markdown

[...] For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

That's a surprising issue 😅.

As you said, users can customise the entrypoint, so for now you can do that too, until we (the team) decides how we should handle our internal diagnostics.

Overwriting the entrypoint is an overkill, we can append additional args:

- repo: https://github.com/biomejs/pre-commitrev: v2.4.4hooks:
- id: biome-checkargs: ["--error-on-warnings"]

From pre-commit docs:

args: (optional) list of additional parameters to pass to the hook.

Sign up for freeto 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.

biome-check hook doesn't fail when there are lints

4 participants

@tyilo@ematipico@matthiask@Guiorgy
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

Add --error-on-warnings to hooks - #59

Open
tyilo wants to merge 1 commit into
biomejs:mainfrom
tyilo:fix-hooks
Open

Add --error-on-warnings to hooks#59
tyilo wants to merge 1 commit into
biomejs:mainfrom
tyilo:fix-hooks

Conversation

@tyilo

Copy link
Copy Markdown
Contributor

Fixes#58

@ematipico

Copy link
Copy Markdown
Member

I don't think this is a good idea. There might be users that want and accepts warnings in their code.

@tyilo

tyilo commented Aug 20, 2025

Copy link
Copy Markdown
ContributorAuthor

I don't think this is a good idea. There might be users that want and accepts warnings in their code.

I don't really see the use-case for running biome lint with pre-commit without this flag. pre-commit won't even show the warnings with a 0 exit code, so I think this makes the hook useless.

Furthermore, the default pre-commit handlers for other linters defaults to failing when there is an unfixed lint.

If the user really wants to have warnings in their code then I don't think pre-commit is the right tool, as the warnings won't even be shown. However, if there is some weird use-case, they can override the entrypoint in their pre-commit config.

@ematipico

ematipico commented Aug 22, 2025

Copy link
Copy Markdown
Member

Still, Biome emits warnings that sometimes fall outside the linter's ecosphere, meaning users can't control them directly until they are fixed. This means that every time we decide to emit a warning, this will fail. For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

As you said, users can customise the entrypoint, so for now you can do that too, until we (the team) decides how we should handle our internal diagnostics.

@tyilo

Copy link
Copy Markdown
ContributorAuthor

Still, Biome emits warnings that sometimes fall outside the linter's ecosphere, meaning users can't control them directly until they are fixed. This means that every time we decide to emit a warning, this will fail. For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

Fair enough. Is there another flag for only exiting with a non-zero exit code when there are lints?
Or maybe biome should by default have a non-zero exit code, when there are lints.

@matthiask

Copy link
Copy Markdown

I always run the pre-commit hook with verbose:true so that I see the warnings even when I don't have any errors. It might make sense to recommend verbose:true -- the warnings are useful but I'd suffer very much if all warnings were errors :-)

@Guiorgy

Guiorgy commented Mar 1, 2026

Copy link
Copy Markdown

[...] For example, we recently landed an update where we emit a internal diagnostic, a warning one, which isn't easily fixable. This could cause a DX friction to the tool.

That's a surprising issue 😅.

As you said, users can customise the entrypoint, so for now you can do that too, until we (the team) decides how we should handle our internal diagnostics.

Overwriting the entrypoint is an overkill, we can append additional args:

- repo: https://github.com/biomejs/pre-commitrev: v2.4.4hooks:
- id: biome-checkargs: ["--error-on-warnings"]

From pre-commit docs:

args: (optional) list of additional parameters to pass to the hook.

Sign up for freeto 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.

biome-check hook doesn't fail when there are lints

4 participants

@tyilo@ematipico@matthiask@Guiorgy