chore(chat-widget): enforce prettier via pre-commit and npm scripts#3234
chore(chat-widget): enforce prettier via pre-commit and npm scripts#3234
Conversation
Adds an explicit prettier dependency, npm scripts (`format`, `format:check`), a `.prettierignore` for Stencil-generated files, and a repo-wide pre-commit hook scoped to the chat widget. Also renames the deprecated `jsxBracketSameLine` key in `.prettierrc.json` to `bracketSameLine`. Without enforcement, contributors' IDEs were silently auto-formatting on save against the existing `.prettierrc.json`, producing large unrelated diffs in otherwise small PRs (e.g. #3196). This makes formatting deterministic across contributors and CI. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Pure formatting commit, no behavior changes. Run `npm run format` to reproduce. Isolated from the prior tooling commit so future PRs touching these files don't mix prettier reformatting with real changes. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
📝 WalkthroughWalkthroughThis pull request introduces Prettier code formatting infrastructure to the Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 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: 3
🧹 Nitpick comments (2)
.pre-commit-config.yaml (1)
49-54: Consider adding a CIformat:checkgate in addition to pre-commit.Pre-commit is helpful but bypassable; a CI check for
components/chat_widgetkeeps formatting enforcement consistent on every PR.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.pre-commit-config.yaml around lines 49 - 54, Add a CI job named "format:check" that runs the same Prettier check as the pre-commit hook to enforce formatting on every PR; implement it to run the equivalent command used by the "prettier" hook (or an npm script like format:check) targeting the same files pattern "components/chat_widget/src/.*\.(ts|tsx|css)$" so the workflow fails when formatting is incorrect and cannot be bypassed by skipping pre-commit.components/chat_widget/package.json (1)
56-56: Pin Prettier to exact version to match pre-commit configuration.
package.jsonuses^3.8.3, while pre-commit is pinned tov3.8.3. Using a caret allows npm to install any version >= 3.8.3 but < 4.0.0, which could lead to inconsistent formatting output between npm scripts and pre-commit hooks.♻️ Suggested change
- "prettier": "^3.8.3", + "prettier": "3.8.3",🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/chat_widget/package.json` at line 56, The Prettier dependency in package.json is declared as "prettier": "^3.8.3", which allows non-exact upgrades and can cause formatting mismatches with the pre-commit hook pinned to v3.8.3; update the dependency to an exact version by replacing the caret range with "prettier": "3.8.3" so npm installs the exact same Prettier used by pre-commit and ensure consistency between npm scripts and hooks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/chat_widget/src/components/ocs-chat/ocs-chat.spec.tsx`:
- Line 17: Install and declare the missing testing/type packages and fix the
runtime TypeScript error: add devDependencies for `@stencil/core`,
`@stencil/core/testing` and appropriate `@types` (e.g., `@types/jest` or `@types/mocha`)
or add module declarations for them so the test environment type-checks; in
ocs-chat.tsx replace the non‑standard Property 'at' usage on ChatMessage[]
(search for usages of ChatMessage[] .at(...), likely in function/variable
handling message retrieval) with a compatible alternative such as indexing with
length-1 (messages[messages.length - 1]) or messages.slice(-1)[0]; update any
tests in ocs-chat.spec.tsx to use Stencil's testing API imports from
`@stencil/core/testing` and ensure the test harness is available in package.json
scripts so stencil tests run in CI.
In `@components/chat_widget/src/components/ocs-chat/ocs-chat.tsx`:
- Around line 1799-1801: The anchor in the OcsChat component (the <a> with
href="https://www.dimagi.com" and target="_blank") is missing the rel attribute;
update that anchor in ocs-chat.tsx (within the OcsChat component/render
function) to include rel="noopener noreferrer" to prevent reverse-tabnabbing
when opening the external link in a new tab.
- Around line 2-18: Update the build_components.yml CI workflow to enforce
linting and typechecking for the chat_widget component by adding two steps after
the existing Test step: a "Lint" step that runs npm run lint with
working-directory set to ./components/chat_widget, and a "Type Check" step that
runs npm run type-check with working-directory set to ./components/chat_widget;
ensure the step names are exactly "Lint" and "Type Check" so they are visible in
job output and place them in the same job that runs the current Test step.
---
Nitpick comments:
In @.pre-commit-config.yaml:
- Around line 49-54: Add a CI job named "format:check" that runs the same
Prettier check as the pre-commit hook to enforce formatting on every PR;
implement it to run the equivalent command used by the "prettier" hook (or an
npm script like format:check) targeting the same files pattern
"components/chat_widget/src/.*\.(ts|tsx|css)$" so the workflow fails when
formatting is incorrect and cannot be bypassed by skipping pre-commit.
In `@components/chat_widget/package.json`:
- Line 56: The Prettier dependency in package.json is declared as "prettier":
"^3.8.3", which allows non-exact upgrades and can cause formatting mismatches
with the pre-commit hook pinned to v3.8.3; update the dependency to an exact
version by replacing the caret range with "prettier": "3.8.3" so npm installs
the exact same Prettier used by pre-commit and ensure consistency between npm
scripts and hooks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 19c44160-db2f-4258-8681-52a77aea1bca
⛔ Files ignored due to path filters (1)
components/chat_widget/package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (16)
.pre-commit-config.yamlcomponents/chat_widget/.prettierignorecomponents/chat_widget/.prettierrc.jsoncomponents/chat_widget/package.jsoncomponents/chat_widget/src/components/ocs-chat/icons.tsxcomponents/chat_widget/src/components/ocs-chat/ocs-chat.csscomponents/chat_widget/src/components/ocs-chat/ocs-chat.spec.tsxcomponents/chat_widget/src/components/ocs-chat/ocs-chat.tsxcomponents/chat_widget/src/components/ocs-chat/ocs-chat_session_handling.spec.tsxcomponents/chat_widget/src/services/chat-session-service.tscomponents/chat_widget/src/services/file-attachment-manager.tscomponents/chat_widget/src/utils/cookies.tscomponents/chat_widget/src/utils/markdown.tscomponents/chat_widget/src/utils/translations.spec.tscomponents/chat_widget/src/utils/translations.tscomponents/chat_widget/src/utils/utils.ts
| // Mock translation manager to return welcome messages | ||
| component.translationManager = new TranslationManager('en', { | ||
| 'content.welcomeMessages': ['Hello from translations!', 'Welcome to our chat.'] | ||
| 'content.welcomeMessages': ['Hello from translations!', 'Welcome to our chat.'], |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd components/chat_widget
npm run lint
npm run test
npm run type-checkRepository: dimagi/open-chat-studio
Length of output: 37413
Address type-checking and testing failures in component widget before submitting.
The lint/test/type-check commands were executed but failed with exit code 2. Specifically:
- Linting completed
- Testing failed: stencil tool is not available in the environment
- Type-checking failed with multiple TypeScript errors, including missing module declarations (
@stencil/core,@stencil/core/testing,@types/jest/@types/mocha) and an actual code issue (Property 'at' does not exist on type 'ChatMessage[]' at line 793 in ocs-chat.tsx)
Resolve the missing dependencies and type errors in components/chat_widget to satisfy validation requirements before this PR is ready.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/chat_widget/src/components/ocs-chat/ocs-chat.spec.tsx` at line 17,
Install and declare the missing testing/type packages and fix the runtime
TypeScript error: add devDependencies for `@stencil/core`, `@stencil/core/testing`
and appropriate `@types` (e.g., `@types/jest` or `@types/mocha`) or add module
declarations for them so the test environment type-checks; in ocs-chat.tsx
replace the non‑standard Property 'at' usage on ChatMessage[] (search for usages
of ChatMessage[] .at(...), likely in function/variable handling message
retrieval) with a compatible alternative such as indexing with length-1
(messages[messages.length - 1]) or messages.slice(-1)[0]; update any tests in
ocs-chat.spec.tsx to use Stencil's testing API imports from
`@stencil/core/testing` and ensure the test harness is available in package.json
scripts so stencil tests run in CI.
| import { Component, Host, h, Prop, State, Element, Watch, Env } from '@stencil/core'; | ||
| import { | ||
| XMarkIcon, | ||
| GripDotsVerticalIcon, PlusWithCircleIcon, ArrowsPointingOutIcon, ArrowsPointingInIcon, | ||
| PaperClipIcon, CheckDocumentIcon, XIcon, OcsWidgetAvatar | ||
| GripDotsVerticalIcon, | ||
| PlusWithCircleIcon, | ||
| ArrowsPointingOutIcon, | ||
| ArrowsPointingInIcon, | ||
| PaperClipIcon, | ||
| CheckDocumentIcon, | ||
| XIcon, | ||
| OcsWidgetAvatar, | ||
| } from './icons'; | ||
| import { renderMarkdownSync as renderMarkdownComplete } from '../../utils/markdown'; | ||
| import { varToPixels } from '../../utils/utils'; | ||
| import {TranslationStrings, TranslationManager, defaultTranslations} from '../../utils/translations'; | ||
| import { | ||
| ChatSessionService, | ||
| ChatMessage, | ||
| MessagePollingHandle, | ||
| TaskPollingHandle | ||
| } from '../../services/chat-session-service'; | ||
| import { | ||
| FileAttachmentManager, | ||
| SelectedFile | ||
| } from '../../services/file-attachment-manager'; | ||
| import { TranslationStrings, TranslationManager, defaultTranslations } from '../../utils/translations'; | ||
| import { ChatSessionService, ChatMessage, MessagePollingHandle, TaskPollingHandle } from '../../services/chat-session-service'; | ||
| import { FileAttachmentManager, SelectedFile } from '../../services/file-attachment-manager'; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find the package.json and understand available npm scripts
find . -name "package.json" -type f | head -20Repository: dimagi/open-chat-studio
Length of output: 1132
🏁 Script executed:
# Look for CI configuration files
find . -name ".github" -o -name ".gitlab-ci.yml" -o -name "tox.ini" -o -name ".travis.yml" | head -10Repository: dimagi/open-chat-studio
Length of output: 498
🏁 Script executed:
# Check if there's a tsconfig.json in the components/chat_widget directory
find ./components/chat_widget -name "tsconfig.json" -o -name "jest.config.*" -o -name ".eslintrc*" | head -20Repository: dimagi/open-chat-studio
Length of output: 106
🏁 Script executed:
# Check root package.json for npm scripts
cat package.json | grep -A 50 '"scripts"'Repository: dimagi/open-chat-studio
Length of output: 1788
🏁 Script executed:
# Check components/chat_widget package.json
cat ./components/chat_widget/package.json | grep -A 30 '"scripts"'Repository: dimagi/open-chat-studio
Length of output: 1235
🏁 Script executed:
# List GitHub Actions workflow files
ls -la ./.github/workflows/Repository: dimagi/open-chat-studio
Length of output: 1364
🏁 Script executed:
# Check the lint_and_test.yml workflow
cat ./.github/workflows/lint_and_test.ymlRepository: dimagi/open-chat-studio
Length of output: 5495
🏁 Script executed:
# Check the build_components.yml workflow
cat ./.github/workflows/build_components.ymlRepository: dimagi/open-chat-studio
Length of output: 1650
Add missing lint and typecheck to CI for components/chat_widget.
The build_components.yml workflow currently runs tests (npm run test, npm run test:security) but is missing lint and typecheck steps. Per coding guidelines, all three must be enforced in CI.
Update build_components.yml
Add these steps after the Test step:
- name: Lint
run: npm run lint
working-directory: ./components/chat_widget
- name: Type Check
run: npm run type-check
working-directory: ./components/chat_widget🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@components/chat_widget/src/components/ocs-chat/ocs-chat.tsx` around lines 2 -
18, Update the build_components.yml CI workflow to enforce linting and
typechecking for the chat_widget component by adding two steps after the
existing Test step: a "Lint" step that runs npm run lint with working-directory
set to ./components/chat_widget, and a "Type Check" step that runs npm run
type-check with working-directory set to ./components/chat_widget; ensure the
step names are exactly "Lint" and "Type Check" so they are visible in job output
and place them in the same job that runs the current Test step.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
1 participant
Product Description
No user-facing change — internal tooling for the chat widget.
Technical Description
Contributors' IDEs were silently auto-formatting chat-widget files on save against the existing
components/chat_widget/.prettierrc.json, producing large unrelated diffs in otherwise small PRs (e.g. #3196). Prettier was the de-facto formatter but had no enforcement: it wasn't a declared dependency, no npm script ran it, and no pre-commit hook checked it.Two commits, kept deliberately separate:
chore(chat-widget): add prettier tooling and pre-commit hook—prettier@^3.8.3as a chat-widget devDependency.format(prettier --write) andformat:check(prettier --check) npm scripts scoped tosrc/**/*.{ts,tsx,css}.components/chat_widget/.prettierignoreexcluding Stencil-generated files (src/components.d.ts,src/components/**/readme.md) and build outputs (dist/,www/,loader/).rbubley/mirrors-prettier@v3.8.3hook to.pre-commit-config.yamlscoped to^components/chat_widget/src/.*\.(ts|tsx|css)$.jsxBracketSameLinekey in.prettierrc.jsontobracketSameLine(prettier 3.x).style(chat-widget): baseline-format all src files with prettier—Pure
npm run formatoutput across 12 pre-existing files. Isolated from commit 1 so future PRs touching these files don't mix prettier reformatting noise with real changes. No behavior changes; reproducible vianpm run format.The chat widget's prettier config (
printWidth: 180,singleQuote: true,bracketSpacing: true,trailingComma: 'all', etc.) is unchanged — only enforcement was added.Migrations
Demo
N/A — tooling only. Verify locally with:
```
cd components/chat_widget
npm install
npm run format:check # should pass
```
Docs and Changelog
🤖 Generated with Claude Code