-
Notifications
You must be signed in to change notification settings - Fork 13k
chore(message-parser): Review build configuration #37737
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
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
|
WalkthroughConverts the message-parser package to a dist-only ES-module build: removes legacy CommonJS entry and legacy build/docs configs, adds a TypeScript webpack build and tsconfig.build, updates package.json exports and Docker COPYs to rely on Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 |
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.
Actionable comments posted: 2
🧹 Nitpick comments (6)
packages/message-parser/tests/unorderedList.test.ts (1)
52-73: Considertest.todo/test.skipinstead of commented-out casesThe two nested/indented list scenarios are fully commented out. If these represent known gaps or flaky behavior, converting them to
test.todoortest.skipcases would make their status more visible in test output and avoid them being forgotten, while still keeping the implementation unchanged.packages/message-parser/tests/codeFence.test.ts (1)
4-4:multiplyhelper matches existing behavior; only minor style nits possibleThe helper does exactly what these tests need (repeat the same value structurally), and works fine with
toMatchObject. If you ever need distinct instances per element, you could switch this to accept a factory callback instead of a value, but that would be beyond this PR’s scope.packages/message-parser/src/utils.ts (1)
183-203: Missing blank line beforelineBreakfunction.Line 200 defines
lineBreakimmediately after the closing ofreducePlainTextswithout a separating blank line, which is inconsistent with the rest of the file's style.[] as Paragraph['value'], ); + export const lineBreak = (): LineBreak => ({packages/message-parser/tests/abuse.test.ts (1)
5-255: Helpers-based abuse expectations look structurally correct but are highly repetitiveThe stress tests now use
paragraph/plain/bold/italic/strikehelpers instead of raw literals, which matches how the AST is built elsewhere and should keep these expectations aligned with internal types. The constructed trees are structurally sound, but the second case has a very large amount of near-identical repeated segments; factoring that pattern into a small generator helper (even local to this test file) would make it easier to maintain or extend these abuse cases later.packages/message-parser/package.json (1)
19-42: Dist-only exports look consistent; verify ESM/module expectations and type emissionRouting
exports["."].default,main, andmoduleall to./dist/messageParser.jsplustypesto./dist/index.d.tsmatches the move to a dist-only package and the new webpack/tsconfig.build setup. Two things to double‑check:
- If
dist/messageParser.jsis not a pure ES module, havingmodulepoint to the same file thatmainuses may surprise bundlers that treatmoduleas an ESM entrypoint. Confirm this matches how downstream consumers import the package.- Ensure the build pipeline actually emits
dist/index.d.ts(e.g., viats-loader+tsconfig.build.jsonor an explicittsc -p tsconfig.build.jsonstep); otherwise the declaredtypespath will break TypeScript consumers.packages/message-parser/tsconfig.build.json (1)
1-10: Explicitly overridenoEmitto guarantee declaration outputThe build tsconfig is correctly scoped to
./srcand setsoutDir/declaration/declarationMapfor emitting types intodist. If the basetsconfig.jsonever enablesnoEmit: true(common for dev/typecheck configs), this extended config would inherit it and prevent declaration files from being written. Consider adding"noEmit": falseundercompilerOptionshere to ensure build behavior is independent of the parent config, and confirm that with the current parent tsconfig declarations are actually emitted intodist.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (52)
ee/apps/account-service/Dockerfile(0 hunks)ee/apps/authorization-service/Dockerfile(0 hunks)ee/apps/ddp-streamer/Dockerfile(0 hunks)ee/apps/omnichannel-transcript/Dockerfile(0 hunks)ee/apps/presence-service/Dockerfile(0 hunks)ee/apps/queue-worker/Dockerfile(0 hunks)ee/apps/stream-hub-service/Dockerfile(0 hunks)packages/message-parser/.prettierrc.js(0 hunks)packages/message-parser/babel.config.js(0 hunks)packages/message-parser/jest.config.ts(1 hunks)packages/message-parser/loaders/pegtransform.js(1 hunks)packages/message-parser/messageParser.js(0 hunks)packages/message-parser/package.json(1 hunks)packages/message-parser/src/definitions.ts(1 hunks)packages/message-parser/src/guards.ts(1 hunks)packages/message-parser/src/index.ts(1 hunks)packages/message-parser/src/typings/peg.d.ts(1 hunks)packages/message-parser/src/utils.ts(2 hunks)packages/message-parser/tests/abuse.test.ts(1 hunks)packages/message-parser/tests/any.test.ts(1 hunks)packages/message-parser/tests/blockquotes.test.ts(1 hunks)packages/message-parser/tests/codeFence.test.ts(1 hunks)packages/message-parser/tests/color.test.ts(1 hunks)packages/message-parser/tests/email.test.ts(1 hunks)packages/message-parser/tests/emoji.test.ts(1 hunks)packages/message-parser/tests/emoticons.test.ts(1 hunks)packages/message-parser/tests/emphasis.test.ts(1 hunks)packages/message-parser/tests/emphasisWithEmoticons.test.ts(1 hunks)packages/message-parser/tests/escaped.test.ts(1 hunks)packages/message-parser/tests/heading.test.ts(1 hunks)packages/message-parser/tests/image.test.ts(1 hunks)packages/message-parser/tests/inlineCode.test.ts(1 hunks)packages/message-parser/tests/inlineCodeStrike.test.ts(1 hunks)packages/message-parser/tests/katex.test.ts(1 hunks)packages/message-parser/tests/lineBreak.test.ts(1 hunks)packages/message-parser/tests/link.test.ts(1 hunks)packages/message-parser/tests/mention.test.ts(1 hunks)packages/message-parser/tests/orderedList.test.ts(1 hunks)packages/message-parser/tests/phoneNumber.test.ts(1 hunks)packages/message-parser/tests/strikethrough.test.ts(1 hunks)packages/message-parser/tests/strongEmphasis.test.ts(1 hunks)packages/message-parser/tests/tasks.test.ts(1 hunks)packages/message-parser/tests/timestamp.test.ts(1 hunks)packages/message-parser/tests/unorderedList.test.ts(1 hunks)packages/message-parser/tests/url.test.ts(1 hunks)packages/message-parser/tsconfig-bundle.json(0 hunks)packages/message-parser/tsconfig.build.json(1 hunks)packages/message-parser/tsconfig.json(1 hunks)packages/message-parser/typedoc.json(0 hunks)packages/message-parser/webpack.config.js(0 hunks)packages/message-parser/webpack.config.ts(1 hunks)packages/peggy-loader/src/index.ts(1 hunks)
💤 Files with no reviewable changes (13)
- packages/message-parser/tsconfig-bundle.json
- packages/message-parser/.prettierrc.js
- ee/apps/queue-worker/Dockerfile
- ee/apps/ddp-streamer/Dockerfile
- packages/message-parser/messageParser.js
- ee/apps/authorization-service/Dockerfile
- ee/apps/account-service/Dockerfile
- ee/apps/stream-hub-service/Dockerfile
- packages/message-parser/babel.config.js
- ee/apps/omnichannel-transcript/Dockerfile
- ee/apps/presence-service/Dockerfile
- packages/message-parser/webpack.config.js
- packages/message-parser/typedoc.json
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/message-parser/tests/escaped.test.tspackages/message-parser/jest.config.tspackages/peggy-loader/src/index.tspackages/message-parser/src/typings/peg.d.tspackages/message-parser/tests/link.test.tspackages/message-parser/src/guards.tspackages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/mention.test.tspackages/message-parser/tests/blockquotes.test.tspackages/message-parser/tests/heading.test.tspackages/message-parser/webpack.config.tspackages/message-parser/tests/katex.test.tspackages/message-parser/tests/any.test.tspackages/message-parser/tests/emphasisWithEmoticons.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/unorderedList.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/color.test.tspackages/message-parser/tests/orderedList.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/loaders/pegtransform.jspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/src/utils.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/src/index.tspackages/message-parser/tests/phoneNumber.test.tspackages/message-parser/src/definitions.tspackages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/emphasis.test.ts
🧠 Learnings (18)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
packages/message-parser/tests/escaped.test.tspackages/message-parser/tests/link.test.tspackages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/blockquotes.test.tspackages/message-parser/tests/heading.test.tspackages/message-parser/tests/any.test.tspackages/message-parser/tests/emphasisWithEmoticons.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/unorderedList.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/emphasis.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
packages/message-parser/tests/escaped.test.tspackages/message-parser/tests/link.test.tspackages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/mention.test.tspackages/message-parser/tests/blockquotes.test.tspackages/message-parser/tests/heading.test.tspackages/message-parser/tests/katex.test.tspackages/message-parser/tests/any.test.tspackages/message-parser/tests/emphasisWithEmoticons.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/unorderedList.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/color.test.tspackages/message-parser/tests/orderedList.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/phoneNumber.test.tspackages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/emphasis.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
packages/message-parser/tests/escaped.test.tspackages/message-parser/tests/link.test.tspackages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/blockquotes.test.tspackages/message-parser/tests/heading.test.tspackages/message-parser/tests/any.test.tspackages/message-parser/tests/emphasisWithEmoticons.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/color.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/emphasis.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
packages/message-parser/tests/escaped.test.tspackages/message-parser/jest.config.tspackages/message-parser/tests/link.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/blockquotes.test.tspackages/message-parser/tsconfig.jsonpackages/message-parser/tests/any.test.tspackages/message-parser/package.jsonpackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/src/utils.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/emphasis.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
packages/message-parser/tests/escaped.test.tspackages/message-parser/jest.config.tspackages/message-parser/tests/link.test.tspackages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/mention.test.tspackages/message-parser/tests/blockquotes.test.tspackages/message-parser/tests/heading.test.tspackages/message-parser/tests/katex.test.tspackages/message-parser/tests/any.test.tspackages/message-parser/tests/emphasisWithEmoticons.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/unorderedList.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/color.test.tspackages/message-parser/tests/orderedList.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/phoneNumber.test.tspackages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/emphasis.test.ts
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Applies to **/*test-case*.md : Define clear, measurable expected results in test cases that can be validated and later converted into automated tests
Applied to files:
packages/message-parser/tests/escaped.test.tspackages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/blockquotes.test.tspackages/message-parser/tests/heading.test.tspackages/message-parser/tests/any.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/unorderedList.test.tspackages/message-parser/tests/orderedList.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
packages/message-parser/tests/link.test.tspackages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/heading.test.tspackages/message-parser/tests/katex.test.tspackages/message-parser/tests/any.test.tspackages/message-parser/tests/emphasisWithEmoticons.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/unorderedList.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/color.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/phoneNumber.test.tspackages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/emphasis.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
packages/message-parser/tests/link.test.tspackages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/mention.test.tspackages/message-parser/tests/blockquotes.test.tspackages/message-parser/tests/heading.test.tspackages/message-parser/tests/katex.test.tspackages/message-parser/tests/any.test.tspackages/message-parser/tests/emphasisWithEmoticons.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/unorderedList.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/color.test.tspackages/message-parser/tests/orderedList.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/phoneNumber.test.tspackages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/emphasis.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
packages/message-parser/tests/link.test.tspackages/message-parser/tests/heading.test.tspackages/message-parser/tests/any.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/phoneNumber.test.tspackages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/emphasis.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
packages/message-parser/tests/link.test.tspackages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/blockquotes.test.tspackages/message-parser/tests/heading.test.tspackages/message-parser/tests/any.test.tspackages/message-parser/tests/emphasisWithEmoticons.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/color.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/phoneNumber.test.tspackages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/emphasis.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
packages/message-parser/tests/link.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/phoneNumber.test.ts
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Reference the test-cases.json file to ensure consistency with existing test case structures and standardized format
Applied to files:
packages/message-parser/tests/emoji.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/unorderedList.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/color.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
packages/message-parser/tests/blockquotes.test.tspackages/message-parser/tests/heading.test.tspackages/message-parser/tests/emphasisWithEmoticons.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/emphasis.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
packages/message-parser/tests/any.test.tspackages/message-parser/tests/timestamp.test.ts
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Applies to **/*test-case*.md : Write test cases in Markdown format following the standardized template with ALL necessary components: Title, Description, Preconditions, Type, Steps, and Expected Result
Applied to files:
packages/message-parser/tests/unorderedList.test.tspackages/message-parser/tests/orderedList.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.beforeAll()` and `test.afterAll()` for setup/teardown in Playwright tests
Applied to files:
packages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/inlineCodeStrike.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
packages/message-parser/tests/strikethrough.test.ts
🧬 Code graph analysis (21)
packages/message-parser/tests/escaped.test.ts (3)
packages/message-parser/src/utils.ts (3)
paragraph(27-27)plain(64-64)bold(29-29)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/src/typings/peg.d.ts (1)
packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)
packages/message-parser/tests/link.test.ts (3)
packages/message-parser/src/utils.ts (9)
paragraph(27-27)link(77-80)plain(64-64)bold(29-29)strike(65-65)italic(62-62)lineBreak(200-203)listItem(130-134)orderedList(126-126)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/lineBreak.test.ts (3)
packages/message-parser/src/utils.ts (3)
paragraph(27-27)plain(64-64)lineBreak(200-203)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/inlineCode.test.ts (3)
packages/message-parser/src/utils.ts (3)
paragraph(27-27)inlineCode(59-59)plain(64-64)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/emphasisWithEmoticons.test.ts (3)
packages/message-parser/src/utils.ts (5)
paragraph(27-27)bold(29-29)plain(64-64)italic(62-62)emoticon(153-157)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/email.test.ts (3)
packages/message-parser/src/utils.ts (3)
paragraph(27-27)link(77-80)plain(64-64)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/tasks.test.ts (1)
packages/message-parser/src/utils.ts (8)
tasks(60-60)task(53-57)plain(64-64)mentionUser(136-139)mentionChannel(121-124)link(77-80)bold(29-29)emoji(141-145)
packages/message-parser/tests/unorderedList.test.ts (3)
packages/message-parser/src/utils.ts (5)
unorderedList(128-128)listItem(130-134)plain(64-64)bold(29-29)emoji(141-145)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/image.test.ts (3)
packages/message-parser/src/utils.ts (3)
paragraph(27-27)image(114-117)plain(64-64)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/color.test.ts (3)
packages/message-parser/src/utils.ts (3)
paragraph(27-27)color(31-34)plain(64-64)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/orderedList.test.ts (1)
packages/message-parser/src/utils.ts (5)
orderedList(126-126)listItem(130-134)plain(64-64)bold(29-29)emoji(141-145)
packages/message-parser/tests/url.test.ts (3)
packages/message-parser/src/utils.ts (4)
autoLink(82-97)plain(64-64)lineBreak(200-203)link(77-80)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/codeFence.test.ts (3)
packages/message-parser/src/utils.ts (4)
code(42-46)codeLine(67-67)plain(64-64)paragraph(27-27)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/emoticons.test.ts (3)
packages/message-parser/src/utils.ts (4)
paragraph(27-27)plain(64-64)emoticon(153-157)bigEmoji(48-51)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/inlineCodeStrike.test.ts (3)
packages/message-parser/src/utils.ts (6)
paragraph(27-27)strike(65-65)inlineCode(59-59)plain(64-64)italic(62-62)bold(29-29)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/abuse.test.ts (3)
packages/message-parser/src/utils.ts (5)
paragraph(27-27)plain(64-64)bold(29-29)italic(62-62)strike(65-65)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/src/utils.ts (1)
packages/message-parser/src/definitions.ts (16)
Types(164-188)Color(37-45)Heading(75-79)Code(64-68)BigEmoji(47-50)Task(26-30)Markup(86-86)Link(137-143)Inlines(211-224)ListItem(16-20)Emoji(52-62)Paragraph(124-127)LineBreak(109-112)KaTeX(114-117)InlineKaTeX(119-122)Timestamp(155-162)
packages/message-parser/tests/strikethrough.test.ts (3)
packages/message-parser/src/utils.ts (8)
paragraph(27-27)strike(65-65)emoji(141-145)plain(64-64)emojiUnicode(147-151)mentionUser(136-139)mentionChannel(121-124)link(77-80)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/phoneNumber.test.ts (3)
packages/message-parser/src/utils.ts (4)
paragraph(27-27)link(77-80)plain(64-64)bold(29-29)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/emphasis.test.ts (1)
packages/message-parser/src/utils.ts (12)
paragraph(27-27)italic(62-62)emoji(141-145)plain(64-64)emojiUnicode(147-151)mentionUser(136-139)mentionChannel(121-124)link(77-80)bold(29-29)strike(65-65)bigEmoji(48-51)inlineCode(59-59)
🔇 Additional comments (44)
packages/message-parser/loaders/pegtransform.js (1)
4-9: Verify format consistency with ES module migration.The file has been reformatted to a single-line arrow function. However, this loader still outputs CommonJS format (
format: 'commonjs'), which may conflict with the PR's stated objective of "updating peggy-loader default format from CommonJS to ES modules."Please clarify: (1) Is this file still used in the new dist-focused build pipeline? (2) If yes, should the format be updated to
'es'or'esm'to align with the ES module strategy?packages/message-parser/tests/heading.test.ts (1)
2-48: Formatting-only changes improve readability while preserving test semantics.The imports are correctly preserved and used throughout the test data. All test cases remain semantically identical, with only whitespace/layout improvements (single-line tuples, consistent alignment). Test logic and behavior are unchanged.
packages/message-parser/tests/lineBreak.test.ts (3)
1-2: Verify theplainimport after API rename toPLAIN.The PR summary mentions an API rename from
PLAIN_TEXTtoPLAIN. Confirm that the importedplainfunction from../src/utilsreflects the correct API and that the internal token type has been updated consistently.[web]
Verify if the plain text API rename in the message-parser package changed the function name or token type from PLAIN_TEXT to PLAIN--- `5-26`: **Test data formatting looks good.** The test data has been reformatted for improved readability with consistent indentation and layout. The three test cases—covering single line breaks, line breaks with trailing newlines, and multiple consecutive line breaks—remain logically intact. --- `28-28`: **Verify that `toMatchObject()` is the correct assertion for this test.** The assertion was updated to use `toMatchObject()`. Confirm this is the intended matcher—it checks that the parsed output contains the expected structure but does not require an exact match. If strict equality is needed, `toEqual()` would be more appropriate. </blockquote></details> <details> <summary>packages/message-parser/tests/inlineCodeStrike.test.ts (3)</summary><blockquote> `1-2`: **Import formatting is clear and maintainable.** The single-line import statement is consistent with the PR's formatting improvements across the codebase. --- `5-11`: **Test data structure is properly formatted.** The test cases are now more compact and readable without altering the test semantics. --- `12-14`: **Verify the assertion method change.** Line 13 uses `toMatchObject` to validate the parsed output. Clarify whether this is intentional: `toMatchObject` performs a lenient match (allowing extra properties), whereas `toEqual` performs strict deep equality. For a parser unit test, strict equality is typically preferred to ensure the exact AST structure matches expected output without extra or modified properties. </blockquote></details> <details> <summary>packages/message-parser/tests/image.test.ts (1)</summary><blockquote> `4-11`: **Test formatting is consistent with PR objectives.** The test data formatting has been updated for improved readability—the first test case is now multiline and properly indented, while the second is condensed to a single line. The test structure remains sound with proper use of `test.each`, appropriate helper functions (`image`, `paragraph`, `plain`), and correct assertion with `expect().toMatchObject()`. No functional changes detected. </blockquote></details> <details> <summary>packages/message-parser/tests/blockquotes.test.ts (2)</summary><blockquote> `1-2`: **LGTM!** Clean imports using utility functions for constructing expected AST structures, which improves readability over raw object literals. --- `4-31`: **LGTM!** Well-structured parameterized tests with clear input/output pairs. The use of utility functions (`paragraph`, `plain`, `quote`, `bold`) for expected AST construction improves maintainability. Test cases cover the key blockquote variants (with/without space after `>`, with inline formatting). </blockquote></details> <details> <summary>packages/message-parser/tests/escaped.test.ts (2)</summary><blockquote> `1-2`: **Test structure and assertion patterns are sound.** The imports match the available utilities, `test.each()` with parameterized inputs is the correct Jest pattern, and `toMatchObject()` is appropriate for deeply comparing AST structures. Also applies to: 4-4, 18-19 --- `5-17`: No action needed. The `plain()` function is correctly defined as `generate('PLAIN_TEXT')` and consistently used throughout the codebase. The PR does not rename `PLAIN_TEXT` to `PLAIN`; the string literal `PLAIN_TEXT` remains in use for type checking and node generation. The test expectations in `escaped.test.ts` correctly match the implementation. </blockquote></details> <details> <summary>packages/message-parser/tests/email.test.ts (1)</summary><blockquote> `5-48`: **Email parsing test table remains correct and improves readability** The refactored `test.each` data keeps the same input/output mappings while making the cases more compact. Positive and negative email cases (including unicode, apostrophes, parentheses, `mailto:` prefix, and malformed domains) are still covered, and the assertion `expect(parse(input)).toMatchObject(output);` remains appropriate with the `paragraph`, `link`, and `plain` helpers. No behavior regressions detected here. Also applies to: 50-50 </blockquote></details> <details> <summary>packages/message-parser/tests/emoticons.test.ts (1)</summary><blockquote> `1-70`: **LGTM! Clean formatting refactor with comprehensive test coverage.** The test cases are well-organized and correctly validate emoticon parsing behavior across normal emojis, big emojis, and edge cases where emojis should not be parsed. The flattened format improves consistency while maintaining semantic correctness. </blockquote></details> <details> <summary>packages/message-parser/tests/unorderedList.test.ts (2)</summary><blockquote> `5-38`: **Unordered list test table is correctly structured with helpers** The first two table entries and the shared `expect(parse(input)).toMatchObject(output);` correctly use `unorderedList`, `listItem`, `plain`, `bold`, and `emoji` to mirror the AST shape produced by the parser while keeping the assertion tolerant via `toMatchObject`. No functional or typing issues here. Also applies to: 75-75 --- `40-51`: **Mixed-bullet list expectations match likely grouping semantics** The third case, with `-` followed by `*` bullets, expecting two separate `unorderedList` nodes (one for the lone `-` item and one for the subsequent `*` items) looks intentional and consistent with typical Markdown grouping rules. The expected structure is coherent and should guard against regressions in list grouping behavior. </blockquote></details> <details> <summary>packages/message-parser/tests/codeFence.test.ts (1)</summary><blockquote> `7-72`: **Code fence fixtures and expectations remain semantically identical** The reflowed `test.each` table preserves input shapes (blank-line counts, languages, and plain-text edge cases) and expected AST structures, and the assertion via `expect(parse(input)).toMatchObject(output)` is unchanged. This looks like a pure formatting/clarity improvement with no behavioral impact. </blockquote></details> <details> <summary>packages/message-parser/tests/mention.test.ts (1)</summary><blockquote> `4-22`: **LGTM!** The test data formatting is more concise and readable. Test inputs and expected outputs remain semantically identical. </blockquote></details> <details> <summary>packages/message-parser/src/guards.ts (1)</summary><blockquote> `3-4`: **LGTM!** Single-line format is appropriate for this concise type guard function. Logic and type safety remain intact. </blockquote></details> <details> <summary>packages/message-parser/src/utils.ts (2)</summary><blockquote> `22-25`: **LGTM!** The `generate` higher-order function is now more concise. The type safety with `keyof Types` and the generic return type remain correct. --- `159-181`: **LGTM!** The `joinEmoji` function formatting is cleaner. The logic correctly handles emoji neighbor detection and emoticon conversion. </blockquote></details> <details> <summary>packages/message-parser/tests/orderedList.test.ts (1)</summary><blockquote> `4-27`: **LGTM!** Test data formatting is improved with consistent indentation. The ordered list test case with mixed content types (plain, bold, emoji) and custom item numbers is correctly structured. </blockquote></details> <details> <summary>packages/message-parser/tests/inlineCode.test.ts (1)</summary><blockquote> `4-12`: **LGTM!** Test data entries are properly formatted as single-line arrays. Test coverage for inline code parsing (URLs, mentions, file extensions) is comprehensive. </blockquote></details> <details> <summary>packages/message-parser/tests/any.test.ts (1)</summary><blockquote> `4-10`: **LGTM!** Formatting is consistent. Test cases appropriately cover plain text parsing including edge cases with unfinished formatting blocks. </blockquote></details> <details> <summary>packages/message-parser/tests/phoneNumber.test.ts (1)</summary><blockquote> `4-27`: **Formatting improvements look good.** Test data entries are consistently formatted. Phone number parsing coverage is comprehensive, including valid formats, edge cases, and non-phone-number inputs. </blockquote></details> <details> <summary>packages/message-parser/src/definitions.ts (1)</summary><blockquote> `104-107`: **Confirm intended naming for `Plain` node discriminant and `Types` key** `Plain` still uses `type: 'PLAIN_TEXT'`, and the `Types` map exposes `PLAIN_TEXT: Plain;`. Helpers (e.g., `plain = generate('PLAIN_TEXT')`) are consistent with this. If the goal is to standardize the public API around a `PLAIN` name instead of `PLAIN_TEXT`, you’ll need to either: - Update the discriminant and `Types` key (plus helpers/guards) accordingly, or - Intentionally keep `PLAIN_TEXT` as the internal discriminant and expose a separate alias at the public surface. Please double‑check which behavior is desired for consumers before merging. Also applies to: 164-188 </blockquote></details> <details> <summary>packages/message-parser/src/typings/peg.d.ts (1)</summary><blockquote> `2-6`: **Typings for `*.pegjs` parser look consistent** The `parse(input: string, options?: ParserOptions) => ASTMessage` declaration and use of `import type` are aligned with the parser entry in `src/index.ts` and should integrate cleanly with TS. </blockquote></details> <details> <summary>packages/message-parser/tsconfig.json (1)</summary><blockquote> `2-7`: **TS config simplification and ES2020 target look fine** Extending the shared `@rocket.chat/tsconfig/server.json` and overriding only `rootDir`, `target: "es2020"`, and `module: "esnext"` is a clean setup and aligns with the dist/ESM‑oriented build. Just ensure the minimum Node/tooling versions across consumers are compatible with ES2020 output. </blockquote></details> <details> <summary>packages/peggy-loader/src/index.ts (1)</summary><blockquote> `21-25`: **ESM default for generated grammars with caller override preserved** Setting `output: 'source'` and `format: 'es'` directly in the options object and then spreading `this.getOptions()` keeps the previous “caller overrides default” behavior while changing the default format from CJS to ES modules, which matches the new ESM/dist direction. No other options semantics change here. </blockquote></details> <details> <summary>packages/message-parser/tests/katex.test.ts (1)</summary><blockquote> `5-22`: **Katex test refactor is formatting‑only** Inputs, expected AST builders (`katex`, `inlineKatex`, `paragraph`, `plain`), and assertion logic are unchanged; only indentation/array layout changed. Tests remain correct and readable. </blockquote></details> <details> <summary>packages/message-parser/tests/color.test.ts (1)</summary><blockquote> `5-17`: **Color tests clearly cover enabled/disabled parsing paths** The new `[input, output, disabledOutput]` structure keeps expectations explicit for both `colors: true` and `colors: false` where relevant, without changing semantics. The conditional on `disabledOutput` is appropriate for the current cases. </blockquote></details> <details> <summary>packages/message-parser/tests/emoji.test.ts (1)</summary><blockquote> `5-35`: **Emoji tests remain semantically identical with clearer tables** The reshaped `test.each` data keeps the same expectations for shortcode and unicode handling (including big‑emoji detection) while making the cases easier to scan. Assertions and helper usage are unchanged. Also applies to: 39-62 </blockquote></details> <details> <summary>packages/message-parser/tests/strikethrough.test.ts (1)</summary><blockquote> `2-47`: **Strikethrough test changes are formatting‑only** The comprehensive set of strikethrough cases (emojis, unicode, mentions, links, and tilde edge cases) is preserved; only imports and test data formatting changed. Behavior and coverage are unchanged. </blockquote></details> <details> <summary>packages/message-parser/tests/link.test.ts (1)</summary><blockquote> `2-387`: **Use of utils helpers keeps link expectations consistent and maintainable** The refactor to build expectations via `paragraph`, `plain`, `link`, list helpers, etc., looks consistent with the AST constructors from `../src/utils` and closely matches the input semantics across the many cases (Slack-style links, query/hash fragments, auto-linked domains, lists, and line breaks). I don’t see any mismatched helper usage or lost edge cases here. </blockquote></details> <details> <summary>packages/message-parser/tests/url.test.ts (1)</summary><blockquote> `5-175`: **autoLink expectations align with helper implementation (including custom domains and invalid TLDs)** The revised tests exercise a good spread of URL shapes (protocol variants, ports, fragments, query strings, unicode domains, custom schemes, and invalid TLDs) and the expectations line up with the `autoLink` implementation: valid URLs become `link(...)`, invalid ones fall back to `plain(...)`, and `customDomains` integration is covered via both `parse(..., { customDomains })` and direct `autoLink` calls. The helper-focused assertions at the bottom (protocol preservation and `//` prefixing) also look correct against the provided implementation. </blockquote></details> <details> <summary>packages/message-parser/tests/emphasisWithEmoticons.test.ts (1)</summary><blockquote> `5-55`: **Emphasis/emoticon interplay is well covered with helper-based AST expectations** These cases accurately exercise the tricky combinations of `*`/`_` emphasis with `:*` and `-_-` emoticons (glued to words, separated by spaces, broken marker sequences), and the expectations built via `paragraph`/`plain`/`bold`/`italic`/`emoticon` line up with the described behavior. The refactor away from literal AST objects to helpers looks correct. </blockquote></details> <details> <summary>packages/message-parser/jest.config.ts (1)</summary><blockquote> `6-12`: **Jest configuration remains consistent and type-safe** The config still uses the shared server preset, correctly wires the `.pegjs` transform, and constrains the shape via `satisfies Config`. No behavioral changes introduced here. </blockquote></details> <details> <summary>packages/message-parser/tests/tasks.test.ts (1)</summary><blockquote> `2-35`: **Task list expectations correctly use task/mention/link/emoji helpers** The refactored expectation builds the task list via `tasks([...task(...)...])` with accurate `status` flags, and composes mentions, channel refs, links, bold formatting, and the `:smile:` emoji using the corresponding helpers from `../src/utils`. This matches the intended parsing of the Markdown checklist input and should track internal AST changes cleanly. </blockquote></details> <details> <summary>packages/message-parser/src/index.ts (1)</summary><blockquote> `8-25`: **LGTM! Formatting improvements align with PR objectives.** The formatting changes improve code consistency and readability without altering functionality. The single-line parse function and reformatted exports are concise and follow TypeScript best practices. </blockquote></details> <details> <summary>packages/message-parser/tests/timestamp.test.ts (1)</summary><blockquote> `1-42`: **LGTM! Test formatting improvements.** The reformatted imports and test data improve consistency across the test suite without changing test behavior or assertions. </blockquote></details> <details> <summary>packages/message-parser/tests/emphasis.test.ts (1)</summary><blockquote> `1-131`: **LGTM! Consistent test formatting.** The consolidated imports and reformatted test data align with the PR's goal of improving code consistency across the test suite without affecting test semantics. </blockquote></details> <details> <summary>packages/message-parser/tests/strongEmphasis.test.ts (1)</summary><blockquote> `1-62`: **LGTM! Test suite formatting consistency.** The formatting changes maintain consistency with other test files in the PR while preserving all test logic and assertions. </blockquote></details> <details> <summary>packages/message-parser/webpack.config.ts (1)</summary><blockquote> `1-40`: **LGTM! Well-structured webpack configuration for ES module output.** The configuration correctly: - Uses ts-loader with dedicated tsconfig.build.json - Includes peggy-loader for grammar files - Enables ES module output with the outputModule experiment - Resolves appropriate extensions This aligns with the PR's goal of simplifying the build process and moving to dist-based distribution. </blockquote></details> </blockquote></details> </details> <!-- This is an auto-generated comment by CodeRabbit for review status -->
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #37737 +/- ##
===========================================
- Coverage 67.70% 67.67% -0.04%
===========================================
Files 3452 3451 -1
Lines 113975 113853 -122
Branches 20940 20922 -18
===========================================
- Hits 77168 77045 -123
Misses 34678 34678
- Partials 2129 2130 +1
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
c0767fa to
be84477
Compare
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.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/message-parser/tests/abuse.test.ts (1)
57-255: Consider generating the repetitive expected output programmatically.The expected output for this test case consists of a repeating pattern (~96 iterations of
bold([italic([plain('**')]), italic([plain('**')])])followed byitalic([bold([plain('_')])])). This could be generated with a loop to significantly reduce file size and improve maintainability.Example approach:
const repeatedPattern = Array.from({ length: 96 }, () => [ bold([italic([plain('**')]), italic([plain('**')])]), italic([bold([plain('_')])]), ]).flat(); // Expected output: [paragraph([...repeatedPattern, bold([italic([plain('**')]), italic([plain('**')])]), plain('__')])]This reduces ~200 lines to ~5 lines while preserving the same test behavior.
packages/message-parser/tests/codeFence.test.ts (1)
4-71: Helper + test data refactor look correct; optional tweak formultiplyThe new
multiplyhelper keeps these tests DRY, and the refactoredtest.eachdata preserves the same code‑fence coverage and expected AST structure, so behavior is unchanged.One small optional improvement:
Array.from({ length: a }, () => element)will reuse the same object reference whenelementis non‑primitive. That’s fine for these immutable expected nodes, but if you ever reusemultiplywith mutable objects or identity‑sensitive structures, consider switching to a factory signature:-const multiply = <T>(a: number, element: T): Array<T> => Array.from({ length: a }, () => element); +const multiply = <T>(count: number, factory: () => T): Array<T> => + Array.from({ length: count }, factory);and calling it as:
[code([codeLine(plain('code')), ...multiply(4, () => codeLine(plain(''))), codeLine(plain('code'))])]packages/message-parser/src/definitions.ts (1)
124-127: Consider simplifying the Exclude in Paragraph value type.Line 126 uses
Array<Exclude<Inlines, Paragraph>>, butParagraphis not included in theInlinesunion (lines 211-224), making theExcluderedundant. You could simplify toArray<Inlines>.export type Paragraph = { type: 'PARAGRAPH'; - value: Array<Exclude<Inlines, Paragraph>>; + value: Array<Inlines>; };packages/message-parser/tests/url.test.ts (2)
132-138: Custom-domain autoLink tests align withcustomDomainsbehaviorThe cases for
http://gitlab.local, baregitlab.local, andinternaltool.intranetcorrectly exerciseparse(input, { customDomains: ['local', 'intranet'] })and the expectations look consistent with that configuration.If you touch this block again, you might also fix the small typo in the
describetitle (“comming” → “coming”), but that’s purely cosmetic.
142-144: Clarify “WITHOUT custom hosts” wording vscustomDomainsoptionThis test asserts that
https://internaltool.testtis left as plain text when callingparse(input, { customDomains: ['local'] }), which makes sense for an invalid TLD.The
describelabel “WITHOUT custom hosts settings” is slightly confusing given thatcustomDomains: ['local']is still passed; if the intent is “without matching custom hosts / invalid TLD”, you might consider tweaking the description (or passing an empty array) to better reflect what’s being exercised.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (52)
ee/apps/account-service/Dockerfile(0 hunks)ee/apps/authorization-service/Dockerfile(0 hunks)ee/apps/ddp-streamer/Dockerfile(0 hunks)ee/apps/omnichannel-transcript/Dockerfile(0 hunks)ee/apps/presence-service/Dockerfile(0 hunks)ee/apps/queue-worker/Dockerfile(0 hunks)ee/apps/stream-hub-service/Dockerfile(0 hunks)packages/message-parser/.prettierrc.js(0 hunks)packages/message-parser/babel.config.js(0 hunks)packages/message-parser/jest.config.ts(1 hunks)packages/message-parser/loaders/pegtransform.js(1 hunks)packages/message-parser/messageParser.js(0 hunks)packages/message-parser/package.json(1 hunks)packages/message-parser/src/definitions.ts(1 hunks)packages/message-parser/src/guards.ts(1 hunks)packages/message-parser/src/index.ts(1 hunks)packages/message-parser/src/typings/peg.d.ts(1 hunks)packages/message-parser/src/utils.ts(2 hunks)packages/message-parser/tests/abuse.test.ts(1 hunks)packages/message-parser/tests/any.test.ts(1 hunks)packages/message-parser/tests/blockquotes.test.ts(1 hunks)packages/message-parser/tests/codeFence.test.ts(1 hunks)packages/message-parser/tests/color.test.ts(1 hunks)packages/message-parser/tests/email.test.ts(1 hunks)packages/message-parser/tests/emoji.test.ts(1 hunks)packages/message-parser/tests/emoticons.test.ts(1 hunks)packages/message-parser/tests/emphasis.test.ts(1 hunks)packages/message-parser/tests/emphasisWithEmoticons.test.ts(1 hunks)packages/message-parser/tests/escaped.test.ts(1 hunks)packages/message-parser/tests/heading.test.ts(1 hunks)packages/message-parser/tests/image.test.ts(1 hunks)packages/message-parser/tests/inlineCode.test.ts(1 hunks)packages/message-parser/tests/inlineCodeStrike.test.ts(1 hunks)packages/message-parser/tests/katex.test.ts(1 hunks)packages/message-parser/tests/lineBreak.test.ts(1 hunks)packages/message-parser/tests/link.test.ts(1 hunks)packages/message-parser/tests/mention.test.ts(1 hunks)packages/message-parser/tests/orderedList.test.ts(1 hunks)packages/message-parser/tests/phoneNumber.test.ts(1 hunks)packages/message-parser/tests/strikethrough.test.ts(1 hunks)packages/message-parser/tests/strongEmphasis.test.ts(1 hunks)packages/message-parser/tests/tasks.test.ts(1 hunks)packages/message-parser/tests/timestamp.test.ts(1 hunks)packages/message-parser/tests/unorderedList.test.ts(1 hunks)packages/message-parser/tests/url.test.ts(1 hunks)packages/message-parser/tsconfig-bundle.json(0 hunks)packages/message-parser/tsconfig.build.json(1 hunks)packages/message-parser/tsconfig.json(1 hunks)packages/message-parser/typedoc.json(0 hunks)packages/message-parser/webpack.config.js(0 hunks)packages/message-parser/webpack.config.ts(1 hunks)packages/peggy-loader/src/index.ts(1 hunks)
💤 Files with no reviewable changes (13)
- ee/apps/account-service/Dockerfile
- packages/message-parser/babel.config.js
- packages/message-parser/tsconfig-bundle.json
- ee/apps/omnichannel-transcript/Dockerfile
- packages/message-parser/messageParser.js
- ee/apps/queue-worker/Dockerfile
- packages/message-parser/.prettierrc.js
- ee/apps/presence-service/Dockerfile
- ee/apps/ddp-streamer/Dockerfile
- ee/apps/authorization-service/Dockerfile
- packages/message-parser/webpack.config.js
- ee/apps/stream-hub-service/Dockerfile
- packages/message-parser/typedoc.json
✅ Files skipped from review due to trivial changes (4)
- packages/message-parser/tsconfig.build.json
- packages/message-parser/loaders/pegtransform.js
- packages/message-parser/tests/heading.test.ts
- packages/message-parser/tests/blockquotes.test.ts
🚧 Files skipped from review as they are similar to previous changes (16)
- packages/message-parser/tests/emoji.test.ts
- packages/message-parser/tests/unorderedList.test.ts
- packages/message-parser/src/guards.ts
- packages/message-parser/tests/any.test.ts
- packages/message-parser/tests/mention.test.ts
- packages/message-parser/tsconfig.json
- packages/message-parser/tests/link.test.ts
- packages/message-parser/tests/lineBreak.test.ts
- packages/message-parser/src/utils.ts
- packages/message-parser/webpack.config.ts
- packages/message-parser/tests/emphasisWithEmoticons.test.ts
- packages/message-parser/tests/emphasis.test.ts
- packages/message-parser/tests/orderedList.test.ts
- packages/message-parser/tests/timestamp.test.ts
- packages/message-parser/src/index.ts
- packages/message-parser/tests/color.test.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/src/typings/peg.d.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/phoneNumber.test.tspackages/message-parser/tests/tasks.test.tspackages/peggy-loader/src/index.tspackages/message-parser/jest.config.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/escaped.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/src/definitions.tspackages/message-parser/tests/katex.test.ts
🧠 Learnings (17)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
packages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/phoneNumber.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/katex.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
packages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/phoneNumber.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/escaped.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/katex.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
packages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/phoneNumber.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/katex.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
packages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/phoneNumber.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/jest.config.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/escaped.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/katex.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
packages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/inlineCodeStrike.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
packages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/escaped.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/inlineCodeStrike.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
packages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/package.jsonpackages/message-parser/tests/tasks.test.tspackages/message-parser/jest.config.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/inlineCodeStrike.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.beforeAll()` and `test.afterAll()` for setup/teardown in Playwright tests
Applied to files:
packages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/inlineCodeStrike.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
packages/message-parser/tests/email.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/phoneNumber.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/inlineCodeStrike.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
packages/message-parser/tests/email.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/tasks.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
packages/message-parser/tests/email.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/image.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Applies to **/*test-case*.md : Define clear, measurable expected results in test cases that can be validated and later converted into automated tests
Applied to files:
packages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/tests/escaped.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
packages/message-parser/tests/phoneNumber.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
packages/message-parser/tests/image.test.tspackages/message-parser/tests/strongEmphasis.test.tspackages/message-parser/tests/strikethrough.test.tspackages/message-parser/tests/inlineCodeStrike.test.ts
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Reference the test-cases.json file to ensure consistency with existing test case structures and standardized format
Applied to files:
packages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
packages/message-parser/tests/strikethrough.test.ts
🧬 Code graph analysis (9)
packages/message-parser/tests/inlineCode.test.ts (3)
packages/message-parser/src/utils.ts (3)
paragraph(27-27)inlineCode(59-59)plain(64-64)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/emoticons.test.ts (3)
packages/message-parser/src/utils.ts (4)
paragraph(27-27)plain(64-64)emoticon(153-157)bigEmoji(48-51)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/src/typings/peg.d.ts (1)
packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)
packages/message-parser/tests/image.test.ts (3)
packages/message-parser/src/utils.ts (3)
paragraph(27-27)image(114-117)plain(64-64)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/strongEmphasis.test.ts (3)
packages/message-parser/src/utils.ts (10)
paragraph(27-27)bold(29-29)emoji(141-145)plain(64-64)emojiUnicode(147-151)mentionUser(136-139)mentionChannel(121-124)link(77-80)italic(62-62)strike(65-65)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/abuse.test.ts (3)
packages/message-parser/src/utils.ts (4)
paragraph(27-27)bold(29-29)italic(62-62)strike(65-65)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/strikethrough.test.ts (3)
packages/message-parser/src/utils.ts (8)
paragraph(27-27)strike(65-65)emoji(141-145)plain(64-64)emojiUnicode(147-151)mentionUser(136-139)mentionChannel(121-124)link(77-80)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/inlineCodeStrike.test.ts (3)
packages/message-parser/src/utils.ts (6)
paragraph(27-27)strike(65-65)inlineCode(59-59)plain(64-64)italic(62-62)bold(29-29)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/katex.test.ts (3)
packages/message-parser/src/utils.ts (4)
katex(205-208)paragraph(27-27)plain(64-64)inlineKatex(210-213)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (27)
packages/message-parser/tests/email.test.ts (1)
5-50: Email test table refactor keeps behavior while improving readabilityThe
test.eachtable is consistent and preserves the previous input→AST expectations (including edge cases and unicode emails), and the singletoMatchObjectassertion keeps the behavior unchanged. This is a clean readability win with no apparent risk.packages/message-parser/src/typings/peg.d.ts (1)
2-6: Typings for PEG parser remain consistent with implementationThe module declaration for
*.pegjsstill exposesparse(input: string, options?: ParserOptions) => ASTMessage, matching the runtimeparseinpackages/message-parser/src/index.ts. The change is purely formatting/indentation, with no impact on the public type surface.packages/message-parser/jest.config.ts (1)
7-11: LGTM! Formatting improvement aligns with PR objectives.The configuration remains functionally identical with improved formatting consistency.
packages/message-parser/tests/inlineCodeStrike.test.ts (3)
2-2: Utils import consolidation looks goodSingle-line import of the AST builders from
../src/utilsis clear and matches existing patterns in this test suite; no issues.
5-11: Test cases reformatted without behavioral changesThe
test.eachdata has been compacted but keeps the same input strings and expected AST shape (paragraph → strike → inlineCode with optional italic/bold nesting). Structure and ordering remain correct.
13-13: Assertion remains appropriate
expect(parse(input)).toMatchObject(output);is still the right choice here to validate the AST shape without over-specifying unrelated fields.packages/message-parser/tests/inlineCode.test.ts (1)
4-12: LGTM! Formatting improves readability.The reformatted test cases are now more concise and consistent, making the test suite easier to scan while preserving all functional behavior.
packages/message-parser/tests/image.test.ts (1)
5-9: Image parsing expectations and matcher update look correctUsing
plain('image')for the labeled case and relying onimage(url)’s default label for the empty‑alt case matches the helper’s contract, and switching totoMatchObjectis a good fit for loosely matching AST node shapes without over‑constraining on extra fields. No further changes needed here.Also applies to: 11-11
packages/message-parser/tests/abuse.test.ts (3)
1-2: LGTM!Clean imports using the utility functions for AST construction, which improves test maintainability over raw object literals.
5-56: LGTM!The first stress test case is well-structured. Using the
paragraph,plain,bold,italic, andstrikehelpers to build the expected AST makes the test more readable and maintainable than hardcoded object literals.
256-258: LGTM!The test runner function is concise and correctly uses
toMatchObjectfor flexible matching against the expected AST structure.packages/message-parser/tests/katex.test.ts (1)
5-21: LGTM! Formatting improvements enhance readability.The reformatted test data structure is more concise and maintains the same test semantics. The simplified array nesting and single-line formatting align with the PR's goal of improving readability and maintainability.
packages/message-parser/tests/strikethrough.test.ts (1)
2-47: Strikethrough test cases remain semantically equivalent and well-structuredThe consolidated import and flattened
test.eachtable keep the expectations readable while preserving the original AST structures for emojis, mentions, links, and plain text;expect(parse(input)).toMatchObject(output)is still an appropriate assertion here. No issues found.packages/message-parser/tests/emoticons.test.ts (1)
1-70: LGTM! Test structure and coverage are excellent.The test file is well-organized with clear test cases covering normal emoticons, big emoticons, and edge cases where emoticons should not be parsed. The reformatted structure is more concise and maintainable while preserving all test scenarios. The organizational comments effectively group related test cases.
packages/message-parser/tests/tasks.test.ts (2)
2-2: LGTM! Utility imports improve test readability.Using utility functions from
../src/utilsto construct the expected AST makes the test data more declarative and easier to maintain.
5-33: LGTM! Compact formatting improves maintainability.The inline structure using utility functions is more concise while maintaining readability. This aligns well with the PR's goal of consistent formatting.
packages/message-parser/src/definitions.ts (1)
164-188: Inconsistency between summary and code regarding PLAIN_TEXT key.The AI summary states "Types mapping key renamed from PLAIN_TEXT to PLAIN within the exported Types interface," but line 167 shows
PLAIN_TEXT: Plain;, notPLAIN: Plain;. The code shows the key remainsPLAIN_TEXT.packages/message-parser/tests/escaped.test.ts (1)
5-19: LGTM!The test data reformatting improves conciseness and readability while preserving all test semantics.
packages/message-parser/tests/phoneNumber.test.ts (1)
5-25: LGTM!The reformatted test data improves consistency and readability across the test suite.
packages/message-parser/tests/strongEmphasis.test.ts (2)
2-2: LGTM!Consolidating utility imports into a single line improves readability.
5-61: LGTM!The reformatted test data enhances consistency and maintainability across the test suite while preserving all test semantics.
packages/message-parser/package.json (3)
19-27: Verify the unified bundle approach.The
mainandmodulefields now both point todist/messageParser.js(previouslymodulepointed todist/messageParser.mjs). This suggests a single ES module bundle approach instead of dual CommonJS/ESM outputs, which aligns with the peggy-loader change to ES format.Confirm the generated
dist/messageParser.jsis compatible with both CommonJS and ES module consumers, or verify that breaking CommonJS compatibility is intentional.
31-33: LGTM!The
filesarray now only includes/dist, removing the legacymessageParser.jsentry point. This aligns with the dist-only packaging approach and coordinates with the Dockerfile updates in the PR.
34-42: LGTM!The addition of
typecheckscript improves type safety validation. The removal of documentation-related scripts aligns with the build simplification objectives.packages/peggy-loader/src/index.ts (1)
21-25: The format change to ES modules is already coordinated with the webpack build.The webpack configuration for
message-parser(the only consumer) is already configured for ES module output (outputModule: true,type: 'module'). Jest uses a separate loader (pegtransform.js), so the format change topeggy-loaderdoes not affect test environments. No action required.Likely an incorrect or invalid review comment.
packages/message-parser/tests/url.test.ts (2)
5-126: URL matrix reformat +toMatchObjectassertion look goodThe table-driven cases still exercise a wide range of URL shapes (schemes, ports, fragments, Unicode, markdown links, long JIRA URLs, etc.), and the expectations remain structurally consistent. Switching the assertion to
toMatchObjectis appropriate here to assert on the AST shape while remaining resilient to additional metadata fields on nodes.Also applies to: 128-128
148-174: autoLink helper behavior is well-covered and consistentThe helper tests nicely pin down protocol preservation (http/https/custom), protocol‑relative URLs, domain inference for bare hostnames, and the fallback to plain text for invalid TLDs. Using
toMatchObjectagainst thelink/plainbuilders keeps them robust while still specific about the intended AST shape.
be84477 to
269520f
Compare
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.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/message-parser/src/definitions.ts (1)
1-4:Blockquoteremains unused; consider removing or wiring it into the AST types.
Blockquoteis still exported but not referenced inBlocks,ASTNode, orTypes, whileQuoteis the block-level type actually used. That makesBlockquoteeffectively dead within this package and a bit misleading as to which node kinds really appear in the AST.If it’s not part of the external API, consider removing it; if it is, consider adding it to the relevant unions so it’s a first-class AST node.
🧹 Nitpick comments (4)
packages/message-parser/tests/abuse.test.ts (1)
57-255: Consider programmatic generation for the repetitive expected output.The expected output contains a highly repetitive 3-line pattern (
bold([...]),italic([...])) repeated ~64 times. While explicit assertions are valuable for abuse tests, you could reduce verbosity with a helper:const repeatedPattern = Array.from({ length: 63 }, () => [ bold([italic([plain('**')]), italic([plain('**')])]), italic([bold([plain('_')])]), ]).flat(); // Then use: paragraph([...repeatedPattern, bold([...]), plain('__')])This would make the test more concise while preserving the same assertion. That said, the explicit form is also acceptable for stress tests where debugging failures benefits from visible structure.
packages/message-parser/tests/codeFence.test.ts (1)
4-71:multiplyhelper reuses the same node instance (okay for tests, but could be clearer)
multiplycurrently repeats the sameelementreference, which is fine withtoMatchObject(structural comparison) but doesn’t reflect distinct AST nodes per blank line. If you ever want closer alignment with the real AST shape, you could switch to a factory-based helper:-const multiply = <T>(a: number, element: T): Array<T> => Array.from({ length: a }, () => element); +const multiply = <T>(a: number, factory: () => T): Array<T> => Array.from({ length: a }, factory); @@ - [code([codeLine(plain('code')), ...multiply(4, codeLine(plain(''))), codeLine(plain('code'))])], + [code([codeLine(plain('code')), ...multiply(4, () => codeLine(plain(''))), codeLine(plain('code'))])], @@ - [code([codeLine(plain('code')), ...multiply(2, codeLine(plain('')))])], + [code([codeLine(plain('code')), ...multiply(2, () => codeLine(plain('')))])],Purely optional; current tests are correct and deterministic.
packages/message-parser/tests/link.test.ts (1)
4-388: Thorough link coverage; consider light structuring of the big table.The
test.eachcases exercise a wide range of link syntaxes and edge cases, and the expectations built viautilsmatch the current AST definitions. From a behavior standpoint this looks solid.If you ever touch this again, consider grouping the table into a few themed
test.eachblocks (basic links, query/hash, lists/line breaks, non-links, nested markup) or extracting some repeated URL constants to slightly reduce cognitive load when scanning this file. Not required for this PR.packages/message-parser/webpack.config.ts (1)
3-40: Webpack config is correct for an ESM library; consider__dirname-based paths.The single-config webpack setup (ts-loader with
tsconfig.build.json, peggy loader,outputModule+library.type: 'module', andmode: 'production') looks correct and aligned with the new dist-only packaging.One small robustness tweak:
resolve('./tsconfig.build.json')/resolve('./src')/resolve('./dist')rely on the current working directory. Usingresolve(__dirname, 'tsconfig.build.json' | 'src' | 'dist')would make this config independent of how webpack is invoked (from the package dir vs workspace root).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (52)
ee/apps/account-service/Dockerfile(0 hunks)ee/apps/authorization-service/Dockerfile(0 hunks)ee/apps/ddp-streamer/Dockerfile(0 hunks)ee/apps/omnichannel-transcript/Dockerfile(0 hunks)ee/apps/presence-service/Dockerfile(0 hunks)ee/apps/queue-worker/Dockerfile(0 hunks)ee/apps/stream-hub-service/Dockerfile(0 hunks)packages/message-parser/.prettierrc.js(0 hunks)packages/message-parser/babel.config.js(0 hunks)packages/message-parser/jest.config.ts(1 hunks)packages/message-parser/loaders/pegtransform.js(1 hunks)packages/message-parser/messageParser.js(0 hunks)packages/message-parser/package.json(1 hunks)packages/message-parser/src/definitions.ts(1 hunks)packages/message-parser/src/guards.ts(1 hunks)packages/message-parser/src/index.ts(1 hunks)packages/message-parser/src/typings/peg.d.ts(1 hunks)packages/message-parser/src/utils.ts(2 hunks)packages/message-parser/tests/abuse.test.ts(1 hunks)packages/message-parser/tests/any.test.ts(1 hunks)packages/message-parser/tests/blockquotes.test.ts(1 hunks)packages/message-parser/tests/codeFence.test.ts(1 hunks)packages/message-parser/tests/color.test.ts(1 hunks)packages/message-parser/tests/email.test.ts(1 hunks)packages/message-parser/tests/emoji.test.ts(1 hunks)packages/message-parser/tests/emoticons.test.ts(1 hunks)packages/message-parser/tests/emphasis.test.ts(1 hunks)packages/message-parser/tests/emphasisWithEmoticons.test.ts(1 hunks)packages/message-parser/tests/escaped.test.ts(1 hunks)packages/message-parser/tests/heading.test.ts(1 hunks)packages/message-parser/tests/image.test.ts(1 hunks)packages/message-parser/tests/inlineCode.test.ts(1 hunks)packages/message-parser/tests/inlineCodeStrike.test.ts(1 hunks)packages/message-parser/tests/katex.test.ts(1 hunks)packages/message-parser/tests/lineBreak.test.ts(1 hunks)packages/message-parser/tests/link.test.ts(1 hunks)packages/message-parser/tests/mention.test.ts(1 hunks)packages/message-parser/tests/orderedList.test.ts(1 hunks)packages/message-parser/tests/phoneNumber.test.ts(1 hunks)packages/message-parser/tests/strikethrough.test.ts(1 hunks)packages/message-parser/tests/strongEmphasis.test.ts(1 hunks)packages/message-parser/tests/tasks.test.ts(1 hunks)packages/message-parser/tests/timestamp.test.ts(1 hunks)packages/message-parser/tests/unorderedList.test.ts(1 hunks)packages/message-parser/tests/url.test.ts(1 hunks)packages/message-parser/tsconfig-bundle.json(0 hunks)packages/message-parser/tsconfig.build.json(1 hunks)packages/message-parser/tsconfig.json(1 hunks)packages/message-parser/typedoc.json(0 hunks)packages/message-parser/webpack.config.js(0 hunks)packages/message-parser/webpack.config.ts(1 hunks)packages/peggy-loader/src/index.ts(1 hunks)
💤 Files with no reviewable changes (13)
- packages/message-parser/.prettierrc.js
- packages/message-parser/typedoc.json
- packages/message-parser/babel.config.js
- packages/message-parser/webpack.config.js
- ee/apps/queue-worker/Dockerfile
- ee/apps/authorization-service/Dockerfile
- ee/apps/omnichannel-transcript/Dockerfile
- ee/apps/ddp-streamer/Dockerfile
- ee/apps/account-service/Dockerfile
- ee/apps/presence-service/Dockerfile
- ee/apps/stream-hub-service/Dockerfile
- packages/message-parser/tsconfig-bundle.json
- packages/message-parser/messageParser.js
✅ Files skipped from review due to trivial changes (7)
- packages/message-parser/tests/strikethrough.test.ts
- packages/message-parser/tests/image.test.ts
- packages/message-parser/tests/tasks.test.ts
- packages/message-parser/jest.config.ts
- packages/message-parser/tests/escaped.test.ts
- packages/message-parser/tests/orderedList.test.ts
- packages/message-parser/tests/emphasis.test.ts
🚧 Files skipped from review as they are similar to previous changes (17)
- packages/message-parser/tests/mention.test.ts
- packages/message-parser/src/index.ts
- packages/message-parser/tests/emoticons.test.ts
- packages/message-parser/src/typings/peg.d.ts
- packages/message-parser/tests/timestamp.test.ts
- packages/message-parser/tests/inlineCodeStrike.test.ts
- packages/message-parser/tests/any.test.ts
- packages/message-parser/tests/heading.test.ts
- packages/message-parser/tests/color.test.ts
- packages/message-parser/tests/blockquotes.test.ts
- packages/message-parser/tsconfig.build.json
- packages/message-parser/tests/lineBreak.test.ts
- packages/message-parser/tests/emphasisWithEmoticons.test.ts
- packages/message-parser/tests/strongEmphasis.test.ts
- packages/message-parser/tests/emoji.test.ts
- packages/message-parser/tests/phoneNumber.test.ts
- packages/message-parser/tests/unorderedList.test.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/katex.test.tspackages/message-parser/src/guards.tspackages/message-parser/loaders/pegtransform.jspackages/message-parser/tests/email.test.tspackages/message-parser/webpack.config.tspackages/message-parser/tests/link.test.tspackages/message-parser/tests/url.test.tspackages/peggy-loader/src/index.tspackages/message-parser/src/utils.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/src/definitions.ts
🧠 Learnings (16)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
packages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/katex.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/link.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
packages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/katex.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/link.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/codeFence.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
packages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/katex.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/link.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
packages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/katex.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/link.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
packages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/link.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Reference the test-cases.json file to ensure consistency with existing test case structures and standardized format
Applied to files:
packages/message-parser/tests/inlineCode.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
packages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/link.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
packages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/link.test.tspackages/message-parser/tests/url.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
packages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/link.test.tspackages/message-parser/tests/url.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
packages/message-parser/tests/inlineCode.test.ts
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Applies to **/*test-case*.md : Define clear, measurable expected results in test cases that can be validated and later converted into automated tests
Applied to files:
packages/message-parser/tests/inlineCode.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
packages/message-parser/tests/email.test.tspackages/message-parser/tests/link.test.tspackages/message-parser/tests/url.test.ts
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Applied to files:
packages/message-parser/webpack.config.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Store commonly used locators in variables/constants for reuse
Applied to files:
packages/message-parser/tests/link.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
packages/message-parser/src/utils.tspackages/message-parser/tsconfig.jsonpackages/message-parser/tests/codeFence.test.tspackages/message-parser/package.json
🧬 Code graph analysis (7)
packages/message-parser/tests/katex.test.ts (3)
packages/message-parser/src/utils.ts (4)
katex(205-208)paragraph(27-27)plain(64-64)inlineKatex(210-213)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/src/guards.ts (2)
packages/message-parser/src/index.ts (1)
isNodeOfType(6-6)packages/message-parser/src/definitions.ts (1)
ASTNode(190-207)
packages/message-parser/tests/email.test.ts (3)
packages/message-parser/src/utils.ts (3)
paragraph(27-27)link(77-80)plain(64-64)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/link.test.ts (3)
packages/message-parser/src/utils.ts (11)
paragraph(27-27)link(77-80)plain(64-64)quote(119-119)bold(29-29)strike(65-65)italic(62-62)lineBreak(200-203)unorderedList(128-128)listItem(130-134)orderedList(126-126)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/url.test.ts (3)
packages/message-parser/src/utils.ts (5)
paragraph(27-27)autoLink(82-97)plain(64-64)lineBreak(200-203)link(77-80)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/src/utils.ts (1)
packages/message-parser/src/definitions.ts (15)
Color(37-45)Heading(75-79)Code(64-68)BigEmoji(47-50)Task(26-30)Markup(86-86)Link(137-143)Inlines(211-224)ListItem(16-20)Emoji(52-62)Paragraph(124-127)LineBreak(109-112)KaTeX(114-117)InlineKaTeX(119-122)Timestamp(155-162)
packages/message-parser/tests/codeFence.test.ts (3)
packages/message-parser/src/utils.ts (4)
code(42-46)codeLine(67-67)plain(64-64)paragraph(27-27)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (10)
packages/message-parser/tests/abuse.test.ts (3)
1-2: LGTM!Imports are correctly structured, bringing in the
parsefunction and AST builder utilities.
5-56: LGTM!The first test case correctly uses AST builder functions to construct the expected deeply-nested output. This is more maintainable than raw object literals and clearly shows the expected parsing structure for this stress test input.
256-258: LGTM!The test execution correctly uses
toMatchObjectwhich allows for flexible matching against the expected AST structure.packages/peggy-loader/src/index.ts (1)
23-24: Clean implementation of ES module output with override capability.The switch from CommonJS to ES module format is correct. The placement of
...this.getOptions()afterformat: 'es'provides a sensible default while allowing consumers to override if needed. The peggy library supports the 'es' format, and the identified consumer (message-parser) is properly configured to handle the output via webpack.packages/message-parser/src/guards.ts (1)
3-4: Type guard remains sound and idiomatic
isNodeOfTypestill correctly checks for a non-null object with a matchingtypefield, and the genericN extends ASTNodediscriminator is preserved. No behavioral changes; this is a safe formatting cleanup.packages/message-parser/tests/inlineCode.test.ts (1)
5-12: Inline code tests: structural assertion and data look correctThe reformatted
test.eachdata preserves inputs/outputs, and usingtoMatchObjectagainst the AST built viaparagraph/inlineCode/plainis appropriate here. No issues spotted.packages/message-parser/tests/katex.test.ts (1)
5-21: KaTeX tests: formatting-only, coverage still solidThe refactored table keeps the same katex inputs/expected AST (block and inline) and continues to assert via
toMatchObjectwith the right options. Looks good.packages/message-parser/tsconfig.json (1)
2-7: TS config aligns with ES module/dist build strategyExtending the shared server config and explicitly targeting
es2020withmodule: "esnext"is consistent with the new ESM/dist-centric build. Nothing problematic here.packages/message-parser/tests/url.test.ts (1)
5-175: URL/autoLink tests remain consistent with helper semanticsThe reorganized cases still thoroughly exercise
autoLinkfor standard, custom, and malformed URLs (including custom domains, unicode, fragments, and line breaks), and the expectations line up with the current helper implementation. Changes are structural/formatting only; behavior is preserved.packages/message-parser/tests/email.test.ts (1)
5-50: Email parsing tests: data consolidation with preserved behaviorThe compacted
test.eachtable still exercises the same email patterns (valid, invalid, unicode, with/withoutmailto:) and matches theautoEmail/linkhelpers viatoMatchObject. No functional regressions here.
269520f to
f7e4d3f
Compare
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.
Actionable comments posted: 3
♻️ Duplicate comments (3)
packages/message-parser/package.json (1)
19-27: Unresolved ESM/CJS compatibility issue persists from prior review.This package exposes
"main"and"module"pointing to the same ESM bundle (./dist/messageParser.js) without declaring"type": "module". Since the package is published publicly on npm, CommonJS consumers usingrequire('@rocket.chat/message-parser')will receive an ESM file and fail due to syntax mismatch.The prior review flagged this concern and remains unaddressed. Recommend either:
- Adding
"type": "module"to signal ESM-only (if no external CJS consumers exist), or- Providing dual exports with
"import"and"require"conditions alongside a separate CJS build.Verify the build strategy and external consumer compatibility before merging.
packages/message-parser/loaders/pegtransform.js (1)
4-9: Critical issue from previous review remains unaddressed.The existing review comment correctly identified that line 7 still uses
format: 'commonjs', which contradicts the PR's objective to convert the package to ES modules. Since the PR updates related infrastructure (webpack config targeting dist, peggy-loader using ES format), this CommonJS output will create a module-format mismatch.The fix suggested in the previous comment remains valid—change line 7 to:
- format: 'commonjs', + format: 'es',packages/message-parser/src/definitions.ts (1)
1-4: UnusedBlockquotealias – either remove or fully integrate it
Blockquoteis exported but not referenced byTypes,ASTNode,Blocks,Root, or any other union in this file. WithQuotealready used as the block quote node, this looks like dead/legacy surface and can confuse consumers.Unless you intend to support a distinct
'BLOCKQUOTE'node (and wire it throughBlocks,Types,ASTNode, parser output, etc.), consider removing this alias.Example removal:
-export type Blockquote = { - type: 'BLOCKQUOTE'; - value: Paragraph[]; -}; -
🧹 Nitpick comments (3)
packages/message-parser/package.json (1)
39-40: Clarify redundant "testunit" script.Both
"test"and"testunit"invokejestidentically. If this is not required by monorepo tooling or a specific workflow, consider removing the duplicate to reduce maintenance overhead.packages/message-parser/tests/link.test.ts (1)
5-387: Expanded link parsing matrix looks correct; consider minor structuring for readabilityThe table exercises a wide variety of link shapes (Slack-style
<url|text>, Markdown with nested formatting, complex query/fragment URLs, custom schemes, file paths, list headers, and explicit non-link cases). Expectations are consistent with the helpers’ semantics, and the multi-line template usage (with/without.trim()) matches the intended parsing behavior; no correctness issues stand out.If this grows further, you might consider splitting into a couple of
describe/test.eachgroups (e.g., “markdown links”, “auto-link detection”, “regressions/edge cases”) or adding a few section marker comments similar to the “Should not parse as link” block to keep it scannable, but it’s non-blocking.packages/message-parser/src/definitions.ts (1)
99-102: RedundantItalicmember inStrike.valueunion
Strike.valueis:value: Array<MarkupExcluding<Strike> | Link | Emoji | UserMention | ChannelMention | InlineCode | Italic | Timestamp>;but
MarkupExcluding<Strike>already includesItalic(sinceMarkupisItalic | Strike | Bold | Plain | ChannelMention). The extra| Italicis redundant and can be dropped for clarity without changing the effective type.Suggested tweak:
-export type Strike = { - type: 'STRIKE'; - value: Array<MarkupExcluding<Strike> | Link | Emoji | UserMention | ChannelMention | InlineCode | Italic | Timestamp>; -}; +export type Strike = { + type: 'STRIKE'; + value: Array<MarkupExcluding<Strike> | Link | Emoji | UserMention | ChannelMention | InlineCode | Timestamp>; +};
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (52)
ee/apps/account-service/Dockerfile(0 hunks)ee/apps/authorization-service/Dockerfile(0 hunks)ee/apps/ddp-streamer/Dockerfile(0 hunks)ee/apps/omnichannel-transcript/Dockerfile(0 hunks)ee/apps/presence-service/Dockerfile(0 hunks)ee/apps/queue-worker/Dockerfile(0 hunks)ee/apps/stream-hub-service/Dockerfile(0 hunks)packages/message-parser/.prettierrc.js(0 hunks)packages/message-parser/babel.config.js(0 hunks)packages/message-parser/jest.config.ts(1 hunks)packages/message-parser/loaders/pegtransform.js(1 hunks)packages/message-parser/messageParser.js(0 hunks)packages/message-parser/package.json(1 hunks)packages/message-parser/src/definitions.ts(1 hunks)packages/message-parser/src/guards.ts(1 hunks)packages/message-parser/src/index.ts(1 hunks)packages/message-parser/src/typings/peg.d.ts(1 hunks)packages/message-parser/src/utils.ts(2 hunks)packages/message-parser/tests/abuse.test.ts(1 hunks)packages/message-parser/tests/any.test.ts(1 hunks)packages/message-parser/tests/blockquotes.test.ts(1 hunks)packages/message-parser/tests/codeFence.test.ts(1 hunks)packages/message-parser/tests/color.test.ts(1 hunks)packages/message-parser/tests/email.test.ts(1 hunks)packages/message-parser/tests/emoji.test.ts(1 hunks)packages/message-parser/tests/emoticons.test.ts(1 hunks)packages/message-parser/tests/emphasis.test.ts(1 hunks)packages/message-parser/tests/emphasisWithEmoticons.test.ts(1 hunks)packages/message-parser/tests/escaped.test.ts(1 hunks)packages/message-parser/tests/heading.test.ts(1 hunks)packages/message-parser/tests/image.test.ts(1 hunks)packages/message-parser/tests/inlineCode.test.ts(1 hunks)packages/message-parser/tests/inlineCodeStrike.test.ts(1 hunks)packages/message-parser/tests/katex.test.ts(1 hunks)packages/message-parser/tests/lineBreak.test.ts(1 hunks)packages/message-parser/tests/link.test.ts(1 hunks)packages/message-parser/tests/mention.test.ts(1 hunks)packages/message-parser/tests/orderedList.test.ts(1 hunks)packages/message-parser/tests/phoneNumber.test.ts(1 hunks)packages/message-parser/tests/strikethrough.test.ts(1 hunks)packages/message-parser/tests/strongEmphasis.test.ts(1 hunks)packages/message-parser/tests/tasks.test.ts(1 hunks)packages/message-parser/tests/timestamp.test.ts(1 hunks)packages/message-parser/tests/unorderedList.test.ts(1 hunks)packages/message-parser/tests/url.test.ts(1 hunks)packages/message-parser/tsconfig-bundle.json(0 hunks)packages/message-parser/tsconfig.build.json(1 hunks)packages/message-parser/tsconfig.json(1 hunks)packages/message-parser/typedoc.json(0 hunks)packages/message-parser/webpack.config.js(0 hunks)packages/message-parser/webpack.config.ts(1 hunks)packages/peggy-loader/src/index.ts(1 hunks)
💤 Files with no reviewable changes (13)
- ee/apps/presence-service/Dockerfile
- ee/apps/omnichannel-transcript/Dockerfile
- ee/apps/authorization-service/Dockerfile
- packages/message-parser/messageParser.js
- packages/message-parser/.prettierrc.js
- packages/message-parser/tsconfig-bundle.json
- packages/message-parser/typedoc.json
- ee/apps/queue-worker/Dockerfile
- ee/apps/stream-hub-service/Dockerfile
- packages/message-parser/webpack.config.js
- ee/apps/account-service/Dockerfile
- packages/message-parser/babel.config.js
- ee/apps/ddp-streamer/Dockerfile
✅ Files skipped from review due to trivial changes (4)
- packages/message-parser/tests/phoneNumber.test.ts
- packages/message-parser/tests/emphasisWithEmoticons.test.ts
- packages/message-parser/tests/tasks.test.ts
- packages/message-parser/tests/katex.test.ts
🚧 Files skipped from review as they are similar to previous changes (22)
- packages/message-parser/tests/emoticons.test.ts
- packages/message-parser/tests/heading.test.ts
- packages/message-parser/tests/escaped.test.ts
- packages/message-parser/webpack.config.ts
- packages/message-parser/tests/color.test.ts
- packages/peggy-loader/src/index.ts
- packages/message-parser/tests/any.test.ts
- packages/message-parser/tests/blockquotes.test.ts
- packages/message-parser/tests/codeFence.test.ts
- packages/message-parser/src/typings/peg.d.ts
- packages/message-parser/tests/mention.test.ts
- packages/message-parser/src/index.ts
- packages/message-parser/tests/strikethrough.test.ts
- packages/message-parser/tests/image.test.ts
- packages/message-parser/tests/orderedList.test.ts
- packages/message-parser/jest.config.ts
- packages/message-parser/tests/lineBreak.test.ts
- packages/message-parser/tests/strongEmphasis.test.ts
- packages/message-parser/tsconfig.build.json
- packages/message-parser/tests/url.test.ts
- packages/message-parser/tests/emphasis.test.ts
- packages/message-parser/tests/unorderedList.test.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/message-parser/tests/link.test.tspackages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/src/guards.tspackages/message-parser/tests/email.test.tspackages/message-parser/src/utils.tspackages/message-parser/loaders/pegtransform.jspackages/message-parser/tests/abuse.test.tspackages/message-parser/src/definitions.tspackages/message-parser/tests/emoji.test.ts
🧠 Learnings (17)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
packages/message-parser/tests/link.test.tspackages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/emoji.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
packages/message-parser/tests/link.test.tspackages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
packages/message-parser/tests/link.test.tspackages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
packages/message-parser/tests/link.test.tspackages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/emoji.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
packages/message-parser/tests/link.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/email.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/page-objects/**/*.ts : Utilize existing page objects pattern from `apps/meteor/tests/e2e/page-objects/`
Applied to files:
packages/message-parser/tests/link.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
packages/message-parser/tests/link.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
packages/message-parser/tests/link.test.tspackages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
packages/message-parser/tests/link.test.tspackages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/inlineCode.test.tspackages/message-parser/tests/email.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Reference the test-cases.json file to ensure consistency with existing test case structures and standardized format
Applied to files:
packages/message-parser/tests/link.test.tspackages/message-parser/tests/inlineCode.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.beforeAll()` and `test.afterAll()` for setup/teardown in Playwright tests
Applied to files:
packages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/inlineCodeStrike.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
packages/message-parser/tests/timestamp.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/package.jsonpackages/message-parser/src/utils.tspackages/message-parser/tsconfig.json
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
packages/message-parser/tests/timestamp.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : All test files must be created in `apps/meteor/tests/e2e/` directory
Applied to files:
packages/message-parser/tests/timestamp.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
packages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/inlineCode.test.ts
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Applies to **/*test-case*.md : Define clear, measurable expected results in test cases that can be validated and later converted into automated tests
Applied to files:
packages/message-parser/tests/inlineCode.test.ts
🧬 Code graph analysis (4)
packages/message-parser/tests/inlineCode.test.ts (3)
packages/message-parser/src/utils.ts (3)
paragraph(27-27)inlineCode(59-59)plain(64-64)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/email.test.ts (3)
packages/message-parser/src/utils.ts (3)
paragraph(27-27)link(77-80)plain(64-64)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/abuse.test.ts (3)
packages/message-parser/src/utils.ts (5)
paragraph(27-27)plain(64-64)bold(29-29)italic(62-62)strike(65-65)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/emoji.test.ts (1)
packages/message-parser/src/utils.ts (5)
paragraph(27-27)emoji(141-145)plain(64-64)bigEmoji(48-51)emojiUnicode(147-151)
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (13)
packages/message-parser/src/guards.ts (1)
3-4: LGTM! Formatting improvement aligns with PR objectives.The single-line formatting makes the type guard more concise while preserving correctness. The logic remains sound: checking object type, null, property existence, then value comparison in the proper order.
packages/message-parser/tests/emoji.test.ts (1)
1-62: LGTM! Test expectations correctly updated.The refactoring consistently applies
bigEmojiwrappers for standalone emojis while keepingparagraphwrappers for emojis mixed with text. Test coverage is comprehensive and expectations align with the updated parser behavior.packages/message-parser/tests/email.test.ts (1)
4-51: LGTM! Test data formatting improved.The test cases have been successfully reformatted to a more compact, single-line format while preserving all input-output mappings and test coverage. This improves maintainability and readability.
packages/message-parser/tests/abuse.test.ts (1)
4-258: LGTM! Excellent test refactoring.The shift from hard-coded literal expectations to programmatically constructed ASTs using utils helpers (paragraph, plain, bold, italic, strike) improves maintainability. If the AST structure changes, these tests will be easier to update.
packages/message-parser/src/utils.ts (3)
60-62: LGTM! New exports added correctly.The new
tasksanditalicexports are properly implemented using thegeneratefunction and expand the public API surface for test utilities and other consumers.
244-268: Previous bug fixed successfully.The
timestampFromIsoTimefunction now correctly calls.getTime()on line 264, fixing the bug from the previous review where it was using the function reference instead of invoking it. The function will now return the correct UNIX timestamp in seconds.
22-25: LGTM! Concise function formatting.The
generatefunction has been reformatted to a more compact single-line arrow function style, improving readability while maintaining functionality.packages/message-parser/tests/timestamp.test.ts (1)
1-42: LGTM! Consistent formatting improvements.The test data and import statements have been successfully reformatted for improved readability, consistent with the broader test suite refactoring in this PR.
packages/message-parser/tests/inlineCodeStrike.test.ts (1)
1-14: LGTM! Test formatting improved.The test cases and imports have been reformatted to a more compact style, improving readability and consistency across the test suite.
packages/message-parser/tests/inlineCode.test.ts (1)
1-12: LGTM! Final test file formatting completed.The test data has been successfully reformatted to match the compact, single-line style used consistently throughout this PR's test suite refactoring.
packages/message-parser/package.json (1)
21-22: Distribution and type metadata updates look solid.The consolidation to
dist/as the sole distribution, addition of"types"to the"."export, and newtypecheckscript are all improvements that support a cleaner, dist-first distribution model and better type safety.Also applies to: 25-26, 32-32, 41-41
packages/message-parser/tests/link.test.ts (1)
2-2: Consolidated utility import is consistent and correctAll imported helpers are used below and the single-line import keeps things concise without introducing unused symbols.
packages/message-parser/src/definitions.ts (1)
211-228:Inlines/Blocks/Rootlayering looks consistentThe new
InlinesandBlocksaliases, and their use inRoot, line up cleanly with the node definitions above (inline-only vs block-level vs BigEmoji root case) and should make the surface easier to consume:export type Inlines = /* inline nodes + Timestamp + InlineKaTeX */; export type Blocks = Code | Heading | Quote | ListItem | Tasks | OrderedList | UnorderedList | LineBreak | KaTeX; export type Root = Array<Paragraph | Blocks> | [BigEmoji];No issues spotted here; the factoring improves readability without changing the apparent shape.
f7e4d3f to
2606f3d
Compare
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.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/message-parser/tsconfig.json (1)
5-5: Remove the redundanttargetoverride.The base
@rocket.chat/tsconfig/server.jsonalready specifies"target": "es2020", so this is redundant. The"module": "esnext"override is necessary and correct for the ES module conversion; keep only that and"rootDir".{ "extends": "@rocket.chat/tsconfig/server.json", "compilerOptions": { "rootDir": ".", - "target": "es2020", "module": "esnext" } }packages/message-parser/tests/tasks.test.ts (1)
35-35: Reconsider weakening the assertion from strict to partial matchUsing
toMatchObjectinstead oftoStrictEqualallows extra properties and looser matching on the parsed output, which can hide regressions if the parser starts emitting unexpected fields or slightly different structures. Unlessparseintentionally returns dynamic/unstable properties that must be ignored, prefer keeping this test strict and asserting exact deep equality:- expect(parse(input)).toMatchObject(output); + expect(parse(input)).toStrictEqual(output);If dynamic fields are expected, consider normalizing/removing them before asserting and documenting which ones are ignored and why.
packages/message-parser/loaders/pegtransform.js (1)
4-9: Confirm whether CommonJS Peggy output is still intentional and usedThis loader still generates
format: 'commonjs'code. Given the rest of the package is moving to an ES-moduledist/messageParser.jspipeline, it’d be good to verify whether this transform is still in use and, if so, whether it should also emit ES module code (or be retired in favor of@rocket.chat/peggy-loader’s ESM path).
🧹 Nitpick comments (1)
packages/message-parser/tests/abuse.test.ts (1)
57-255: Consider generating the repetitive expected output programmatically.The expected output contains ~63 repetitions of the same pattern, spanning nearly 200 lines. This could be more maintainable if generated:
const repeatedPattern = Array.from({ length: 62 }, () => [ bold([italic([plain('**')]), italic([plain('**')])]), italic([bold([plain('_')])]), ]).flat(); // Second test case expected output: [paragraph([...repeatedPattern, bold([italic([plain('**')]), italic([plain('**')])]), plain('__')])]This would make the test easier to maintain and the repetition count explicit. However, if verbosity is intentional for ensuring exact match visibility during test failures, the current approach is acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (52)
ee/apps/account-service/Dockerfile(0 hunks)ee/apps/authorization-service/Dockerfile(0 hunks)ee/apps/ddp-streamer/Dockerfile(0 hunks)ee/apps/omnichannel-transcript/Dockerfile(0 hunks)ee/apps/presence-service/Dockerfile(0 hunks)ee/apps/queue-worker/Dockerfile(0 hunks)ee/apps/stream-hub-service/Dockerfile(0 hunks)packages/message-parser/.prettierrc.js(0 hunks)packages/message-parser/babel.config.js(0 hunks)packages/message-parser/jest.config.ts(1 hunks)packages/message-parser/loaders/pegtransform.js(1 hunks)packages/message-parser/messageParser.js(0 hunks)packages/message-parser/package.json(1 hunks)packages/message-parser/src/definitions.ts(1 hunks)packages/message-parser/src/guards.ts(1 hunks)packages/message-parser/src/index.ts(1 hunks)packages/message-parser/src/typings/peg.d.ts(1 hunks)packages/message-parser/src/utils.ts(2 hunks)packages/message-parser/tests/abuse.test.ts(1 hunks)packages/message-parser/tests/any.test.ts(1 hunks)packages/message-parser/tests/blockquotes.test.ts(1 hunks)packages/message-parser/tests/codeFence.test.ts(1 hunks)packages/message-parser/tests/color.test.ts(1 hunks)packages/message-parser/tests/email.test.ts(1 hunks)packages/message-parser/tests/emoji.test.ts(1 hunks)packages/message-parser/tests/emoticons.test.ts(1 hunks)packages/message-parser/tests/emphasis.test.ts(1 hunks)packages/message-parser/tests/emphasisWithEmoticons.test.ts(1 hunks)packages/message-parser/tests/escaped.test.ts(1 hunks)packages/message-parser/tests/heading.test.ts(1 hunks)packages/message-parser/tests/image.test.ts(1 hunks)packages/message-parser/tests/inlineCode.test.ts(1 hunks)packages/message-parser/tests/inlineCodeStrike.test.ts(1 hunks)packages/message-parser/tests/katex.test.ts(1 hunks)packages/message-parser/tests/lineBreak.test.ts(1 hunks)packages/message-parser/tests/link.test.ts(1 hunks)packages/message-parser/tests/mention.test.ts(1 hunks)packages/message-parser/tests/orderedList.test.ts(1 hunks)packages/message-parser/tests/phoneNumber.test.ts(1 hunks)packages/message-parser/tests/strikethrough.test.ts(1 hunks)packages/message-parser/tests/strongEmphasis.test.ts(1 hunks)packages/message-parser/tests/tasks.test.ts(1 hunks)packages/message-parser/tests/timestamp.test.ts(1 hunks)packages/message-parser/tests/unorderedList.test.ts(1 hunks)packages/message-parser/tests/url.test.ts(1 hunks)packages/message-parser/tsconfig-bundle.json(0 hunks)packages/message-parser/tsconfig.build.json(1 hunks)packages/message-parser/tsconfig.json(1 hunks)packages/message-parser/typedoc.json(0 hunks)packages/message-parser/webpack.config.js(0 hunks)packages/message-parser/webpack.config.ts(1 hunks)packages/peggy-loader/src/index.ts(1 hunks)
💤 Files with no reviewable changes (13)
- ee/apps/omnichannel-transcript/Dockerfile
- ee/apps/account-service/Dockerfile
- packages/message-parser/tsconfig-bundle.json
- ee/apps/queue-worker/Dockerfile
- packages/message-parser/webpack.config.js
- packages/message-parser/babel.config.js
- ee/apps/ddp-streamer/Dockerfile
- packages/message-parser/messageParser.js
- ee/apps/presence-service/Dockerfile
- packages/message-parser/typedoc.json
- packages/message-parser/.prettierrc.js
- ee/apps/authorization-service/Dockerfile
- ee/apps/stream-hub-service/Dockerfile
✅ Files skipped from review due to trivial changes (3)
- packages/message-parser/tests/heading.test.ts
- packages/message-parser/tests/phoneNumber.test.ts
- packages/message-parser/src/definitions.ts
🚧 Files skipped from review as they are similar to previous changes (24)
- packages/message-parser/tests/katex.test.ts
- packages/message-parser/tests/blockquotes.test.ts
- packages/message-parser/tests/link.test.ts
- packages/message-parser/tsconfig.build.json
- packages/message-parser/tests/strikethrough.test.ts
- packages/message-parser/tests/unorderedList.test.ts
- packages/message-parser/tests/timestamp.test.ts
- packages/message-parser/tests/inlineCodeStrike.test.ts
- packages/message-parser/tests/color.test.ts
- packages/message-parser/tests/emphasisWithEmoticons.test.ts
- packages/message-parser/tests/escaped.test.ts
- packages/message-parser/jest.config.ts
- packages/message-parser/tests/email.test.ts
- packages/message-parser/tests/lineBreak.test.ts
- packages/message-parser/tests/codeFence.test.ts
- packages/message-parser/tests/url.test.ts
- packages/message-parser/tests/inlineCode.test.ts
- packages/message-parser/tests/image.test.ts
- packages/message-parser/src/index.ts
- packages/message-parser/tests/any.test.ts
- packages/message-parser/tests/strongEmphasis.test.ts
- packages/message-parser/src/utils.ts
- packages/message-parser/tests/orderedList.test.ts
- packages/message-parser/src/typings/peg.d.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/peggy-loader/src/index.tspackages/message-parser/webpack.config.tspackages/message-parser/src/guards.tspackages/message-parser/tests/tasks.test.tspackages/message-parser/loaders/pegtransform.jspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/emphasis.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/mention.test.tspackages/message-parser/tests/abuse.test.ts
🧠 Learnings (17)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Applied to files:
packages/message-parser/webpack.config.tspackages/message-parser/tsconfig.json
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
packages/message-parser/tests/tasks.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/mention.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
packages/message-parser/tests/tasks.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/emphasis.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/mention.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
packages/message-parser/tests/tasks.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/emphasis.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/mention.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
packages/message-parser/tests/tasks.test.tspackages/message-parser/tests/emphasis.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
packages/message-parser/tests/tasks.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/emphasis.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/mention.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
packages/message-parser/tests/tasks.test.tspackages/message-parser/tests/emphasis.test.tspackages/message-parser/tests/mention.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
packages/message-parser/tests/tasks.test.tspackages/message-parser/tests/emphasis.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/abuse.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
packages/message-parser/tests/tasks.test.tspackages/message-parser/tests/emphasis.test.tspackages/message-parser/tsconfig.jsonpackages/message-parser/package.json
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
packages/message-parser/tests/tasks.test.ts
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Reference the test-cases.json file to ensure consistency with existing test case structures and standardized format
Applied to files:
packages/message-parser/tests/tasks.test.tspackages/message-parser/tests/emoji.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
packages/message-parser/tests/tasks.test.tspackages/message-parser/tests/emphasis.test.ts
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Applies to **/*test-case*.md : Define clear, measurable expected results in test cases that can be validated and later converted into automated tests
Applied to files:
packages/message-parser/tests/tasks.test.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
packages/message-parser/tsconfig.json
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
packages/message-parser/tsconfig.json
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.{ts,tsx,js} : Avoid code comments in the implementation
Applied to files:
packages/message-parser/tsconfig.json
🧬 Code graph analysis (3)
packages/message-parser/src/guards.ts (2)
packages/message-parser/src/index.ts (1)
isNodeOfType(6-6)packages/message-parser/src/definitions.ts (1)
ASTNode(190-207)
packages/message-parser/tests/tasks.test.ts (1)
packages/message-parser/src/utils.ts (8)
tasks(60-60)task(53-57)plain(64-64)mentionUser(136-139)mentionChannel(121-124)link(77-80)bold(29-29)emoji(141-145)
packages/message-parser/tests/emphasis.test.ts (3)
packages/message-parser/src/utils.ts (12)
paragraph(27-27)italic(62-62)emoji(141-145)plain(64-64)emojiUnicode(147-151)mentionUser(136-139)mentionChannel(121-124)link(77-80)bold(29-29)strike(65-65)bigEmoji(48-51)inlineCode(59-59)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (14)
packages/message-parser/tests/abuse.test.ts (3)
1-2: LGTM!Clean imports using the parser and utility functions for AST construction.
5-56: LGTM!Well-structured stress test with programmatic AST construction. The deeply nested expected output correctly validates the parser's handling of complex mixed formatting with special characters.
256-258: LGTM!Appropriate use of
test.eachfor parameterized testing andtoMatchObjectfor AST validation.packages/message-parser/src/guards.ts (1)
3-4: LGTM! Clean type guard implementation.The reformatted single-line signature improves readability while maintaining correct type guard logic. The runtime checks properly validate the discriminated union before narrowing the type.
packages/message-parser/tests/emoji.test.ts (1)
4-62: LGTM! Clean test refactoring with improved readability.The test cases are well-structured and comprehensive, covering:
- Custom emoji syntax (
:emoji:) with various edge cases- Unicode emoji support including complex sequences (ZWJ, skin tones)
- Proper distinction between
bigEmoji(emoji-only content) andparagraph(mixed content)- Edge cases for spacing, colons in numbers, and quoted text
The formatting improvements align perfectly with the PR's goal of more concise and maintainable test code.
packages/message-parser/tests/tasks.test.ts (1)
2-33: Updated AST helpers andtaskswrapper usage look correctThe expanded utils import and the new expected structure using
tasks([...])plustask,mentionUser,mentionChannel,link,bold, andemojimatch the utils API and produce a clear top‑level[tasks([...])]AST shape for this checklist. No issues from a correctness or readability standpoint.packages/message-parser/tests/emphasis.test.ts (3)
2-15: Import block is clean and fully utilizedAll imported helpers from
../src/utilsare used in the test matrix, and the refactored multi-line import improves readability without changing behavior.
17-128: Reformatted test matrix preserves behaviorThe
test.eachtable is now more consistently formatted, but inputs and expected AST helper calls remain semantically identical, keeping coverage and behavior unchanged while improving scanability.
129-131: Assertion pattern is appropriate for AST comparisonsUsing
expect(parse(...)).toMatchObject(output)is suitable for these structured AST expectations and aligns with the preferredexpect-based style for TS tests based on prior learnings.packages/peggy-loader/src/index.ts (1)
21-25: ES module format migration is correct.The change from
'commonjs'to'es'aligns with the PR's objective. The webpack configuration uses peggy-loader without passing any options, so the current spread order poses no risk of format overrides. The implementation correctly provides a default ES format that users can still override if needed via loader options.Note:
packages/message-parser/loaders/pegtransform.js(Jest transformer) separately usesformat: 'commonjs'for test processing, creating a dual code path. This is a separate transformation layer and doesn't affect the peggy-loader change, but should be addressed in a follow-up if tests require ES module output consistency.packages/message-parser/tests/mention.test.ts (1)
5-21: Test data reformat is semantically neutral and clearerThe
test.eachcases keep the same input/output pairs, just reformatted for readability. Assertions and coverage are unchanged.packages/message-parser/webpack.config.ts (1)
1-40: Webpack config matches the new dist-based ESM buildEntry, loaders, resolve extensions,
mode: 'production',experiments.outputModule, andlibrary.type: 'module'are all consistent with producing an ESM bundle atdist/messageParser.jsfor this package.packages/message-parser/tests/emoticons.test.ts (1)
5-68: Emoticon test matrix refactor preserves behaviorThe
test.eachtable is flattened/reformatted but still exercises the same “normal emoji”, “BigEmoji”, and negative cases with identical expected AST shapes. Assertion logic is unchanged.packages/message-parser/package.json (1)
19-41: Dist-based exports and tooling updates align with the new buildPointing
exports["."].default,main,module, andtypesatdist/*plus restrictingfilesto/distmatches the new webpack output. The addedtypecheckscript is also a good safeguard for TS types in this package.Also applies to: 53-53
2606f3d to
ee25f1b
Compare
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.
Actionable comments posted: 1
♻️ Duplicate comments (3)
packages/message-parser/package.json (2)
19-26: Note: ESM/CJS compatibility concern already flagged.The export configuration changes (pointing
mainandmoduleto the samedist/messageParser.jsfile withlibrary.type: 'module'in webpack) have already been identified as a potential breaking change for external CommonJS consumers in previous review comments.
53-54: Note: ESLint version incompatibility already flagged.The version mismatch between
eslint@~8.45.0and@typescript-eslint/parser@~5.60.1(which requires ESLint^8.57.0 || ^9.0.0) has already been identified in previous review comments.packages/message-parser/tsconfig.json (1)
2-6:targetoverride is redundant with the base tsconfig
@rocket.chat/tsconfig/server.jsonalready sets"target": "es2020", so this local override is just noise; you can keeprootDirandmoduleonly:{ "extends": "@rocket.chat/tsconfig/server.json", "compilerOptions": { "rootDir": ".", - "target": "es2020", "module": "esnext" } }Same suggestion as in the earlier review; feel free to keep it if you prefer the explicitness here.
🧹 Nitpick comments (1)
packages/peggy-loader/src/index.ts (1)
21-25: Consider enforcingformat: 'es'(andoutput: 'source') by reordering the spreadWith
...this.getOptions()last, anyformatoroutputprovided in the loader options will override the intended defaults ('es'and'source'). If this loader is meant to always emit ES-module source (which seems to be the goal for message-parser), consider putting the spread first and fixed options last:- return peggy.generate(grammarContent, { - output: 'source', - format: 'es', - ...this.getOptions(), - }); + return peggy.generate(grammarContent, { + ...this.getOptions(), + output: 'source', + format: 'es', + });That keeps other options configurable while making the ES-module/source output guarantees harder to accidentally override. If overriding
format/outputis intentional for other consumers, current code is fine.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Disabled knowledge base sources:
- Jira integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (52)
ee/apps/account-service/Dockerfile(0 hunks)ee/apps/authorization-service/Dockerfile(0 hunks)ee/apps/ddp-streamer/Dockerfile(0 hunks)ee/apps/omnichannel-transcript/Dockerfile(0 hunks)ee/apps/presence-service/Dockerfile(0 hunks)ee/apps/queue-worker/Dockerfile(0 hunks)ee/apps/stream-hub-service/Dockerfile(0 hunks)packages/message-parser/.prettierrc.js(0 hunks)packages/message-parser/babel.config.js(0 hunks)packages/message-parser/jest.config.ts(1 hunks)packages/message-parser/loaders/pegtransform.js(1 hunks)packages/message-parser/messageParser.js(0 hunks)packages/message-parser/package.json(1 hunks)packages/message-parser/src/definitions.ts(1 hunks)packages/message-parser/src/guards.ts(1 hunks)packages/message-parser/src/index.ts(1 hunks)packages/message-parser/src/typings/peg.d.ts(1 hunks)packages/message-parser/src/utils.ts(2 hunks)packages/message-parser/tests/abuse.test.ts(1 hunks)packages/message-parser/tests/any.test.ts(1 hunks)packages/message-parser/tests/blockquotes.test.ts(1 hunks)packages/message-parser/tests/codeFence.test.ts(1 hunks)packages/message-parser/tests/color.test.ts(1 hunks)packages/message-parser/tests/email.test.ts(1 hunks)packages/message-parser/tests/emoji.test.ts(1 hunks)packages/message-parser/tests/emoticons.test.ts(1 hunks)packages/message-parser/tests/emphasis.test.ts(1 hunks)packages/message-parser/tests/emphasisWithEmoticons.test.ts(1 hunks)packages/message-parser/tests/escaped.test.ts(1 hunks)packages/message-parser/tests/heading.test.ts(1 hunks)packages/message-parser/tests/image.test.ts(1 hunks)packages/message-parser/tests/inlineCode.test.ts(1 hunks)packages/message-parser/tests/inlineCodeStrike.test.ts(1 hunks)packages/message-parser/tests/katex.test.ts(1 hunks)packages/message-parser/tests/lineBreak.test.ts(1 hunks)packages/message-parser/tests/link.test.ts(1 hunks)packages/message-parser/tests/mention.test.ts(1 hunks)packages/message-parser/tests/orderedList.test.ts(1 hunks)packages/message-parser/tests/phoneNumber.test.ts(1 hunks)packages/message-parser/tests/strikethrough.test.ts(1 hunks)packages/message-parser/tests/strongEmphasis.test.ts(1 hunks)packages/message-parser/tests/tasks.test.ts(1 hunks)packages/message-parser/tests/timestamp.test.ts(1 hunks)packages/message-parser/tests/unorderedList.test.ts(1 hunks)packages/message-parser/tests/url.test.ts(1 hunks)packages/message-parser/tsconfig-bundle.json(0 hunks)packages/message-parser/tsconfig.build.json(1 hunks)packages/message-parser/tsconfig.json(1 hunks)packages/message-parser/typedoc.json(0 hunks)packages/message-parser/webpack.config.js(0 hunks)packages/message-parser/webpack.config.ts(1 hunks)packages/peggy-loader/src/index.ts(1 hunks)
💤 Files with no reviewable changes (13)
- ee/apps/omnichannel-transcript/Dockerfile
- ee/apps/authorization-service/Dockerfile
- packages/message-parser/tsconfig-bundle.json
- packages/message-parser/messageParser.js
- packages/message-parser/babel.config.js
- ee/apps/ddp-streamer/Dockerfile
- ee/apps/queue-worker/Dockerfile
- packages/message-parser/webpack.config.js
- ee/apps/presence-service/Dockerfile
- packages/message-parser/.prettierrc.js
- packages/message-parser/typedoc.json
- ee/apps/stream-hub-service/Dockerfile
- ee/apps/account-service/Dockerfile
✅ Files skipped from review due to trivial changes (6)
- packages/message-parser/tests/inlineCode.test.ts
- packages/message-parser/src/typings/peg.d.ts
- packages/message-parser/loaders/pegtransform.js
- packages/message-parser/tests/strikethrough.test.ts
- packages/message-parser/tests/heading.test.ts
- packages/message-parser/tests/phoneNumber.test.ts
🚧 Files skipped from review as they are similar to previous changes (20)
- packages/message-parser/tests/strongEmphasis.test.ts
- packages/message-parser/tests/link.test.ts
- packages/message-parser/tests/tasks.test.ts
- packages/message-parser/src/utils.ts
- packages/message-parser/tests/mention.test.ts
- packages/message-parser/tests/blockquotes.test.ts
- packages/message-parser/webpack.config.ts
- packages/message-parser/tests/email.test.ts
- packages/message-parser/tests/emphasis.test.ts
- packages/message-parser/src/index.ts
- packages/message-parser/tests/url.test.ts
- packages/message-parser/tests/unorderedList.test.ts
- packages/message-parser/tests/katex.test.ts
- packages/message-parser/jest.config.ts
- packages/message-parser/tests/orderedList.test.ts
- packages/message-parser/tests/emphasisWithEmoticons.test.ts
- packages/message-parser/tests/image.test.ts
- packages/message-parser/tsconfig.build.json
- packages/message-parser/tests/any.test.ts
- packages/message-parser/src/definitions.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js}
📄 CodeRabbit inference engine (.cursor/rules/playwright.mdc)
**/*.{ts,tsx,js}: Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Avoid code comments in the implementation
Files:
packages/message-parser/tests/escaped.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/src/guards.tspackages/peggy-loader/src/index.tspackages/message-parser/tests/color.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/timestamp.test.ts
🧠 Learnings (19)
📓 Common learnings
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Group related tests in the same file
Applied to files:
packages/message-parser/tests/escaped.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/color.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/timestamp.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure tests run reliably in parallel without shared state conflicts
Applied to files:
packages/message-parser/tests/escaped.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/color.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/timestamp.test.ts
📚 Learning: 2025-12-10T21:00:54.909Z
Learnt from: KevLehman
Repo: RocketChat/Rocket.Chat PR: 37091
File: ee/packages/abac/jest.config.ts:4-7
Timestamp: 2025-12-10T21:00:54.909Z
Learning: Rocket.Chat monorepo: Jest testMatch pattern '<rootDir>/src/**/*.spec.(ts|js|mjs)' is valid in this repo and used across multiple packages (e.g., packages/tools, ee/packages/omnichannel-services). Do not flag it as invalid in future reviews.
Applied to files:
packages/message-parser/tests/escaped.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/color.test.tspackages/message-parser/package.jsonpackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tsconfig.jsonpackages/message-parser/tests/emoji.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.step()` for complex test scenarios to improve organization in Playwright tests
Applied to files:
packages/message-parser/tests/escaped.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/color.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/timestamp.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Utilize Playwright fixtures (`test`, `page`, `expect`) for consistency in test files
Applied to files:
packages/message-parser/tests/escaped.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/color.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/emoji.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/timestamp.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.{ts,tsx,js} : Write concise, technical TypeScript/JavaScript with accurate typing in Playwright tests
Applied to files:
packages/message-parser/tests/escaped.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/package.jsonpackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tsconfig.jsonpackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/timestamp.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `expect` matchers for assertions (`toEqual`, `toContain`, `toBeTruthy`, `toHaveLength`, etc.) instead of `assert` statements in Playwright tests
Applied to files:
packages/message-parser/tests/escaped.test.tspackages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/color.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/timestamp.test.ts
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Applies to **/*test-case*.md : Define clear, measurable expected results in test cases that can be validated and later converted into automated tests
Applied to files:
packages/message-parser/tests/escaped.test.tspackages/message-parser/tests/lineBreak.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Maintain test isolation between test cases in Playwright tests
Applied to files:
packages/message-parser/tests/emoticons.test.tspackages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/color.test.tspackages/message-parser/tests/inlineCodeStrike.test.tspackages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/timestamp.test.ts
📚 Learning: 2025-11-24T17:08:26.531Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/test-cases.mdc:0-0
Timestamp: 2025-11-24T17:08:26.531Z
Learning: Reference the test-cases.json file to ensure consistency with existing test case structures and standardized format
Applied to files:
packages/message-parser/tests/emoticons.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.{ts,spec.ts} : Follow Page Object Model pattern consistently in Playwright tests
Applied to files:
packages/message-parser/tests/lineBreak.test.tspackages/message-parser/tests/abuse.test.tspackages/message-parser/tests/color.test.tspackages/message-parser/tests/timestamp.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to **/*.spec.ts : Use descriptive test names that clearly communicate expected behavior in Playwright tests
Applied to files:
packages/message-parser/tests/abuse.test.tspackages/message-parser/tests/color.test.tspackages/message-parser/tests/inlineCodeStrike.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Prefer web-first assertions (`toBeVisible`, `toHaveText`, etc.) in Playwright tests
Applied to files:
packages/message-parser/tests/abuse.test.tspackages/message-parser/tests/inlineCodeStrike.test.ts
📚 Learning: 2025-09-19T15:15:04.642Z
Learnt from: rodrigok
Repo: RocketChat/Rocket.Chat PR: 36991
File: apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts:219-221
Timestamp: 2025-09-19T15:15:04.642Z
Learning: The Federation_Matrix_homeserver_domain setting in apps/meteor/server/services/federation/infrastructure/rocket-chat/adapters/Settings.ts is part of the old federation system and is being deprecated/removed, so configuration issues with this setting should not be flagged for improvement.
Applied to files:
packages/message-parser/tsconfig.json
📚 Learning: 2025-11-10T19:06:20.146Z
Learnt from: MartinSchoeler
Repo: RocketChat/Rocket.Chat PR: 37408
File: apps/meteor/client/views/admin/ABAC/useRoomAttributeOptions.tsx:53-69
Timestamp: 2025-11-10T19:06:20.146Z
Learning: In the Rocket.Chat repository, do not provide suggestions or recommendations about code sections marked with TODO comments. The maintainers have already identified these as future work and external reviewers lack the full context about implementation plans and timing.
Applied to files:
packages/message-parser/tsconfig.json
📚 Learning: 2025-11-19T12:32:29.696Z
Learnt from: d-gubert
Repo: RocketChat/Rocket.Chat PR: 37547
File: packages/i18n/src/locales/en.i18n.json:634-634
Timestamp: 2025-11-19T12:32:29.696Z
Learning: Repo: RocketChat/Rocket.Chat
Context: i18n workflow
Learning: In this repository, new translation keys should be added to packages/i18n/src/locales/en.i18n.json only; other locale files are populated via the external translation pipeline and/or fall back to English. Do not request adding the same key to all locale files in future reviews.
Applied to files:
packages/message-parser/tsconfig.json
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Use `test.beforeAll()` and `test.afterAll()` for setup/teardown in Playwright tests
Applied to files:
packages/message-parser/tests/codeFence.test.tspackages/message-parser/tests/timestamp.test.ts
📚 Learning: 2025-11-24T17:08:17.065Z
Learnt from: CR
Repo: RocketChat/Rocket.Chat PR: 0
File: .cursor/rules/playwright.mdc:0-0
Timestamp: 2025-11-24T17:08:17.065Z
Learning: Applies to apps/meteor/tests/e2e/**/*.spec.ts : Ensure clean state for each test execution in Playwright tests
Applied to files:
packages/message-parser/tests/timestamp.test.ts
🧬 Code graph analysis (8)
packages/message-parser/tests/escaped.test.ts (4)
packages/message-parser/src/utils.ts (3)
paragraph(27-27)plain(64-64)bold(29-29)packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (1)
input(149-159)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/lineBreak.test.ts (3)
packages/message-parser/src/utils.ts (3)
paragraph(27-27)plain(64-64)lineBreak(200-203)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/abuse.test.ts (4)
packages/message-parser/src/utils.ts (5)
paragraph(27-27)plain(64-64)bold(29-29)italic(62-62)strike(65-65)packages/fuselage-ui-kit/src/surfaces/FuselageSurfaceRenderer.tsx (1)
input(149-159)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/src/guards.ts (2)
packages/message-parser/src/index.ts (1)
isNodeOfType(6-6)packages/message-parser/src/definitions.ts (1)
ASTNode(190-207)
packages/message-parser/tests/color.test.ts (3)
packages/message-parser/src/utils.ts (3)
paragraph(27-27)color(31-34)plain(64-64)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/inlineCodeStrike.test.ts (2)
packages/message-parser/src/utils.ts (6)
paragraph(27-27)strike(65-65)inlineCode(59-59)plain(64-64)italic(62-62)bold(29-29)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)
packages/message-parser/tests/emoji.test.ts (3)
packages/message-parser/src/utils.ts (5)
paragraph(27-27)emoji(141-145)plain(64-64)bigEmoji(48-51)emojiUnicode(147-151)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
packages/message-parser/tests/codeFence.test.ts (3)
packages/message-parser/src/utils.ts (4)
code(42-46)codeLine(67-67)plain(64-64)paragraph(27-27)packages/message-parser/src/index.ts (2)
parse(18-18)parse(22-22)packages/message-parser/src/typings/peg.d.ts (1)
parse(6-6)
⏰ 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). (3)
- GitHub Check: 📦 Build Packages
- GitHub Check: CodeQL-Build
- GitHub Check: CodeQL-Build
🔇 Additional comments (17)
packages/message-parser/src/guards.ts (1)
3-4: LGTM!The single-line formatting improves conciseness while preserving the type guard's correctness and readability. The logic properly checks object structure before the type assertion, ensuring type safety.
packages/message-parser/tests/inlineCodeStrike.test.ts (1)
2-2: LGTM!The formatting changes improve readability by consolidating the imports and making the test data more concise. The test logic remains intact, correctly validating inline code parsing with strikethrough, italic, and bold combinations.
Also applies to: 5-11, 13-13
packages/message-parser/tests/color.test.ts (1)
4-18: LGTM! Test restructuring improves readability.The inline tuple format makes the test cases more concise while maintaining correctness. The hex color conversions are accurate (e.g.,
0xcc= 204,0xc7= 199), invalid formats correctly fall back to plain text, and the conditional logic fordisabledOutputappropriately handles cases where color parsing behavior differs based on thecolorsoption.packages/message-parser/tests/codeFence.test.ts (2)
4-4: Typedmultiplyhelper is clear and correctThe generic one-liner using
Array.fromis type-safe and keeps the test data concise; no issues from a correctness or readability standpoint.
7-69: Reformattedtest.eachcases preserve semanticsThe restructured code‑fence fixtures still cover the same scenarios (blank lines, languages, non‑code indented fences, markdown inside fences), and the expectations built with
code/codeLine/plain/paragraphremain consistent with the parser AST helpers.packages/message-parser/tests/emoticons.test.ts (1)
4-70: LGTM! Test data reformatting improves readability.The test cases have been successfully restructured to a more concise inline format while preserving all original test coverage for emoticons, BigEmojis, and edge cases. The formatting aligns with the PR's objective of improving test maintainability.
packages/message-parser/tests/abuse.test.ts (1)
4-258: LGTM! AST-based test expectations are semantically correct.The conversion from string literals to programmatic AST construction using
paragraph,plain,bold,italic, andstrikehelpers maintains the original test semantics. The second test case (lines 57-255) is intentionally verbose to stress-test the parser with deeply nested formatting patterns, which aligns with the file's "abuse testing" purpose.packages/message-parser/tests/lineBreak.test.ts (1)
4-29: LGTM! Line break test coverage is comprehensive.The test cases correctly validate single and multiple consecutive line breaks, with expected outputs accurately reflecting the parser's behavior. The format is clear and maintainable.
packages/message-parser/package.json (1)
40-41: Added typecheck script improves development workflow.The addition of the
typecheckscript provides a convenient way to validate TypeScript types without emitting files, complementing the existing build and test scripts.packages/message-parser/tests/escaped.test.ts (2)
5-17: Escaping and test data refactor looks consistentDouble-checked the string literals and escape sequences (including the
split('').join('\\')case and the backtick example): runtime inputs and expectedparagraph/plain/boldvalues still align with the intended escaping behavior. The refactor to inline/compact test data doesn’t change semantics and remains readable.
19-19: Confirm intentional use oftoMatchObjectfor AST assertionsUsing
toMatchObjectis appropriate for asserting AST shape while ignoring extra properties, but it’s less strict thantoEqual. Please confirm this matcher choice is intentional across the message-parser tests so future structural changes don’t silently bypass expectations.packages/message-parser/tests/emoji.test.ts (2)
5-34: Colon-code emoji expectations and big-emoji detection look consistent and well-coveredThe table cleanly distinguishes emoji-only messages (correctly wrapped in
bigEmoji([...]), allowing whitespace/newlines) from mixed content that stays inparagraph([...]), and the boundary cases around quotes, numbers, and embedded colons exercise the parser logic thoroughly. I don’t see issues with the updated expectations.
39-61: Unicode emoji matrix aligns with big-emoji semantics and covers complex sequencesThe unicode tests mirror the colon-code semantics (emoji-only →
bigEmoji, mixed text/emoji →paragraph) and include a solid spread of ZWJ sequences, skin tones, variation selectors, and multi-emoji runs. This should give good confidence in the unicode parsing behavior; no changes requested.packages/message-parser/tests/timestamp.test.ts (4)
2-2: Import changes look correct
timestampFromHoursis now imported and used below; no unused imports or behavioral changes introduced.
5-9: Base timestamp parsing tests remain soundThe happy‑path timestamp cases (
<t:…>and<t:…:R>, plus inline usage) still assert the same AST structure; the switch totoMatchObjectand compact literals does not change semantics.
13-17: Invalid/unsupported timestamp tests preservedThe cases for unsupported style (
I) and too‑short timestamp (<t:17>) still validate that the input is treated as plain text, so behavior is unchanged.
20-24: Formatting around nested markup is unchangedThe behavior for timestamps inside strike/bold remains consistent (timestamp recognized inside
~…~, but not inside*…*), only formatting was compacted.
| ['<t:2025-07-22T10:00:00.000+00:00:R>', [paragraph([timestamp((Date.parse('2025-07-22T10:00:00.000+00:00') / 1000).toString(), 'R')])]], | ||
| ['<t:2025-07-22T10:00:00.000+00:00:R>', [paragraph([timestamp((Date.parse('2025-07-22T10:00:00.000+00:00') / 1000).toString(), 'R')])]], | ||
| ['<t:2025-07-22T10:00:00.000+00:00:R>', [paragraph([timestamp((Date.parse('2025-07-22T10:00:00.000+00:00') / 1000).toString(), 'R')])]], | ||
| ['<t:2025-07-22T10:00:00+00:00:R>', [paragraph([timestamp((Date.parse('2025-07-22T10:00:00+00:00') / 1000).toString(), 'R')])]], | ||
| ['<t:10:00:00+00:00:R>', [paragraph([timestamp(timestampFromHours('10', '00', '00', '+00:00'), 'R')])]], | ||
| ['<t:10:00+00:00:R>', [paragraph([timestamp(timestampFromHours('10', '00', '00', '+00:00'), 'R')])]], | ||
| ['<t:10:00:05+00:00>', [paragraph([timestamp(timestampFromHours('10', '00', '05', '+00:00'), 't')])]], | ||
| ['<t:10:00+00:00>', [paragraph([timestamp(timestampFromHours('10', '00', '00'), 't')])]], | ||
|
|
||
| [ | ||
| '<t:2025-07-24T20:19:58.154+00:00:R>', | ||
| [ | ||
| paragraph([ | ||
| timestamp( | ||
| ((Date.parse('2025-07-24T20:19:58.154+00:00') / 1000) | 0).toString(), | ||
| 'R', | ||
| ), | ||
| ]), | ||
| ], | ||
| ], | ||
| [ | ||
| '<t:2025-07-24T20:19:58.154+00:00:R>', | ||
| [paragraph([timestamp(((Date.parse('2025-07-24T20:19:58.154+00:00') / 1000) | 0).toString(), 'R')])], | ||
| ], | ||
| ])('parses %p', (input, output) => { | ||
| expect(parse(input)).toMatchObject(output); | ||
| expect(parse(input)).toMatchObject(output); | ||
| }); |
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.
🧩 Analysis chain
🏁 Script executed:
cd packages/message-parser && find . -name "timestamp.test.ts" -type fRepository: RocketChat/Rocket.Chat
Length of output: 91
🏁 Script executed:
cd packages/message-parser && cat -n tests/timestamp.test.ts | sed -n '20,50p'Repository: RocketChat/Rocket.Chat
Length of output: 1656
🏁 Script executed:
cd packages/message-parser && rg "timestampFromHours" --type ts -A 3 -B 1Repository: RocketChat/Rocket.Chat
Length of output: 1413
🏁 Script executed:
cd packages/message-parser && git log --oneline tests/timestamp.test.ts | head -5Repository: RocketChat/Rocket.Chat
Length of output: 126
🏁 Script executed:
cd packages/message-parser && git show HEAD:tests/timestamp.test.ts | sed -n '27,42p'Repository: RocketChat/Rocket.Chat
Length of output: 281
🏁 Script executed:
cd packages/message-parser && cat -n src/utils.ts | grep -A 15 "timestampFromHours"Repository: RocketChat/Rocket.Chat
Length of output: 589
🏁 Script executed:
cd packages/message-parser && git show HEAD:packages/message-parser/tests/timestamp.test.ts | sed -n '27,42p'Repository: RocketChat/Rocket.Chat
Length of output: 1252
🏁 Script executed:
cd packages/message-parser && git log --oneline -p tests/timestamp.test.ts | head -100Repository: RocketChat/Rocket.Chat
Length of output: 2504
Three identical test cases at lines 27–29 and inconsistent timezone handling at line 34
Lines 27–29 contain three identical test cases that appear intentional but may represent test coverage gaps or unintentional duplication. Confirm whether these should be distinct test inputs or deduplicated.
Line 34 calls timestampFromHours('10', '00', '00') without passing the timezone argument for input '<t:10:00+00:00>', whereas line 33 explicitly passes '+00:00' for the semantically similar input '<t:10:00:05+00:00>'. Although timestampFromHours defaults timezone to an empty string, this inconsistency means line 34's expectation may diverge from the parser's actual behavior when parsing the +00:00 timezone in the input. Align the timezone argument usage across these cases.
Proposed changes (including videos or screenshots)
This pull request refactors the
packages/message-parserpackage to simplify its build process and codebase, remove legacy and unnecessary files, and improve code formatting and type definitions. The changes affect both the package itself and several service Dockerfiles that consume it, ensuring consistency and reducing maintenance overhead.Build and Packaging Simplification
messageParser.jsand updated all service Dockerfiles to stop copying this file, relying instead on the main bundle indist/messageParser.js.package.jsonto point all main/module/type exports to thedistdirectory, removed unused scripts and Babel dependencies, and added atypecheckscript for TypeScript..prettierrc.jsandbabel.config.js.Code Formatting and Style
src/to use more concise and consistent formatting, such as single-line arrow functions and simplified conditional logic.Type Definitions Improvements
src/definitions.tsby collapsing multi-line union types and improving readability.Test Cleanup
Issue(s)
ARCH-1908
Steps to test or reproduce
Further comments
Summary by CodeRabbit
Refactor
Chores
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.