Development: Replace graphemer and markdown-it-highlightjs with platform code#12915
Conversation
…orm/native code graphemer: the single use in the Monaco editor's custom backspace action (grapheme-cluster aware delete) now uses the native Intl.Segmenter, which is supported across all target browsers and the Node test runtime. markdown-it-highlightjs: inline a tiny local markdown-it plugin that sets the highlight hook to delegate to highlight.js and wraps the fence / code_block renderers to keep the `hljs` CSS class. This reproduces the exact markup the plugin emitted (the vs.css / monokai.css themes target `.hljs`), so syntax highlighting is unchanged in light and dark mode. The highlight.js engine stays a direct dependency. Verified: tsc compile, ESLint, markdown conversion spec (81 tests, incl. fenced code blocks) and the Monaco editor spec (52 tests, incl. the combined-emoji backspace cluster deletion) all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
github-actions
Bot
added
the
client
There was a problem hiding this comment.
Pull request overview
This PR reduces client dependencies by replacing two small third-party utilities with platform/native equivalents while aiming to preserve existing behavior in markdown code highlighting and Monaco backspace grapheme deletion.
Changes:
- Replace
graphemerusage in the Monaco custom backspace action withIntl.Segmenter-based grapheme segmentation. - Replace
markdown-it-highlightjswith a local markdown-it plugin that delegates highlighting to the existinghighlight.jsdependency and preserves thehljsCSS class. - Remove the two dependencies from
package.jsonand updatepnpm-lock.yamlaccordingly.
Reviewed changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/main/webapp/app/foundation/util/markdown.conversion.util.ts | Introduces local highlight.js integration and renderer wrapping to replace markdown-it-highlightjs. |
| src/main/webapp/app/editor/monaco-editor/monaco-editor.component.ts | Switches grapheme splitting for backspace deletion from Graphemer to Intl.Segmenter. |
| package.json | Removes graphemer and markdown-it-highlightjs dependencies. |
| pnpm-lock.yaml | Reflects dependency removals (and associated lockfile metadata updates). |
Files not reviewed (1)
- pnpm-lock.yaml: Generated file
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughTwo npm dependencies are removed by substituting native or inline alternatives. ChangesDependency Removal and Inline Replacements
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
End-to-End Test Results
❌ Failed Tests
Flakiness Scores for Failed Tests
Test Strategy: Running all tests (configuration or infrastructure changes detected) Overall: ❌ E2E tests failed 🔗 Workflow Run · 📊 Test Report |
krusche
changed the title
Development: Replace graphemer and markdown-it-highlightjs with platform code
|
@Claudia-Anthropica review |
|
@krusche acknowledged, queuing this up for immediate review! |
…hemer-and-highlightjs # Conflicts: # pnpm-lock.yaml
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Line 27: Remove the three unrelated dependencies that were accidentally added
to package.json: `@dagrejs/dagre` (line 27), `d3-shape` (line 49), and
`@types/d3-shape` (line 97). These graph and shape libraries are not part of the
PR's stated objective of removing `graphemer` and `markdown-it-highlightjs`, and
should be removed to keep the changes focused and aligned with the PR scope.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 72eadb26-9763-4562-9a34-6965ca348203
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yaml
📒 Files selected for processing (1)
package.json
github-project-automation
Bot
moved this from Work In Progress
to Ready For Review
in Artemis Development
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@package.json`:
- Line 27: Remove the three unrelated dependencies that were accidentally added
to package.json: `@dagrejs/dagre` (line 27), `d3-shape` (line 49), and
`@types/d3-shape` (line 97). These graph and shape libraries are not part of the
PR's stated objective of removing `graphemer` and `markdown-it-highlightjs`, and
should be removed to keep the changes focused and aligned with the PR scope.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 72eadb26-9763-4562-9a34-6965ca348203
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml,!**/*.yaml
📒 Files selected for processing (1)
package.json
🛑 Comments failed to post (1)
package.json (1)
27-27:
⚠️ Potential issue | 🟠 Major | ⚖️ Poor tradeoffUnrelated dependency additions detected.
The additions of
@dagrejs/dagre,d3-shape, and@types/d3-shapeare not mentioned in the PR objectives, which focus exclusively on removinggraphemerandmarkdown-it-highlightjs. These graph and shape libraries appear unrelated to the grapheme segmentation or markdown highlighting changes.Were these dependencies accidentally included from another branch or PR? Consider removing them to keep this PR focused on its stated objective.
Also applies to: 49-49, 97-97
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@package.json` at line 27, Remove the three unrelated dependencies that were accidentally added to package.json: `@dagrejs/dagre` (line 27), `d3-shape` (line 49), and `@types/d3-shape` (line 97). These graph and shape libraries are not part of the PR's stated objective of removing `graphemer` and `markdown-it-highlightjs`, and should be removed to keep the changes focused and aligned with the PR scope.
- highlightWithHljs: check hljs.getLanguage(language) before calling hljs.highlight() so unknown languages produce an escaped-HTML fallback without throwing an exception (avoids relying on try/catch for common cases). - Intl.Segmenter: hoist the instance to a module-level const (GRAPHEME_SEGMENTER) so it is created once rather than on every Backspace keystroke. - addHljsClass: kept unchanged since the string replacement is a verbatim port of the former library's approach and guarantees identical output; switching to token-attribute manipulation would risk subtle markup differences. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
@Claudia-Anthropica review |
|
@krusche got it, but I'm tapped out right now (my weekly quota is maxed). Queued — I'll pick it up when capacity resets at 2026-06-17 14:25 UTC (~2d 22h from now). |
krusche
temporarily deployed
to
playwright-e2e-tests
GitHub Actions
Inactive
krusche
temporarily deployed
to
playwright-e2e-tests
GitHub Actions
Inactive
Claudia-Anthropica
left a comment
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@krusche The code replacement itself looks solid and matches the old wrapper behavior. I left one cleanup note for stale dependency references; client build/style/tests are green, and the remaining red CI looks unrelated to this client-only change.
…erences Remove both packages from the allowedCommonJsDependencies allow-list in angular.json and update the markdown support docs to reference highlight.js instead of the removed markdown-it-highlightjs plugin. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
FelixTJDietrich
dismissed stale reviews from Claudia-Anthropica and coderabbitai[bot]
via
c185421
FelixTJDietrich
temporarily deployed
to
playwright-e2e-tests
GitHub Actions
Inactive
Claudia-Anthropica
left a comment
Claudia-Anthropica
left a comment
There was a problem hiding this comment.
@krusche The dependency cleanup looks good now. I compared the markdown rendering against the removed plugin for the relevant cases and the targeted markdown/Monaco Vitest files pass locally, so approving.
helios-aet
Bot
temporarily deployed
to
artemis-test2.artemis.cit.tum.de
FelixTJDietrich
left a comment
FelixTJDietrich
left a comment
There was a problem hiding this comment.
Tested on TS2, works as expected. I applied the review comments to fix a stale reference.
krusche
temporarily deployed
to
playwright-e2e-tests
GitHub Actions
Inactive
Rephrase the code-highlighting JSDoc to describe current behavior instead of narrating the migration away from markdown-it-highlightjs, so the docs stay accurate once the former dependency is forgotten. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
FelixTJDietrich
dismissed stale reviews from coderabbitai[bot], Claudia-Anthropica, and themself
via
1f172dd
github-project-automation
Bot
moved this from Ready For Review
to Merged
in Artemis Development
FelixTJDietrich
temporarily deployed
to
playwright-e2e-tests
GitHub Actions
Inactive
Summary
Removes two small client dependencies by replacing them with platform/native equivalents, with no behavior change:
graphemer-> nativeIntl.Segmentermarkdown-it-highlightjs-> a tiny local markdown-it plugin over the already-presenthighlight.jsengineMotivation and Context
Part of an incremental client dependency cleanup. Both libraries wrap functionality that the platform (or an already-installed engine) provides directly.
Description
graphemer (1 site). The Monaco editor registers a custom backspace action that deletes one grapheme cluster at a time (so emoji and combined characters delete as a unit and collaborative edits stay minimal). The single
Graphemer().splitGraphemes(...)call is replaced byIntl.Segmenterwithgranularity: 'grapheme', which is supported in all target browsers and the Node test runtime.markdown-it-highlightjs (1 site). The plugin was used with default options (
auto,code,ignoreIllegals). It is replaced by a small local markdown-it plugin (markdownItHighlightjsinmarkdown.conversion.util.ts) that:md.options.highlightto delegate tohighlight.js(auto-detect when no language, explicit language otherwise, escaped-HTML fallback on error), andfence/code_blockrenderers to add thehljsCSS class.This reproduces the exact markup the plugin produced. The
vs.css/monokai.csshighlight.js themes target the.hljsclass, so highlighting is unchanged in both themes.highlight.jsremains a direct dependency.Steps for Testing
Verified locally:
pnpm run compile(fulltsc) passes.markdown.conversion.util.spec.ts: 81 tests pass, including "should convert fenced code blocks".monaco-editor.component.spec.ts: 52 tests pass, including "delete a combined emoji entirely on backspace" and "delete combined emojis one cluster at a time".To test manually: render markdown containing fenced code blocks (with and without a language) and confirm syntax highlighting in light and dark mode; in the Monaco editor, type an emoji / combined character and press Backspace to confirm the whole cluster is removed.
Client
🤖 Generated with Claude Code
Summary by CodeRabbit