Replace stapler-adjunct-codemirror (CM2) with modern CodeMirror 6#26379
Replace stapler-adjunct-codemirror (CM2) with modern CodeMirror 6#26379LakshyaBagani wants to merge 1 commit intojenkinsci:masterfrom
Conversation
Remove the legacy stapler-adjunct-codemirror dependency (CodeMirror 2) and replace it with CodeMirror 6 from NPM, resolving issue jenkinsci#18689. - Add CM6 as a separate webpack entry point to avoid bloating vendors.js - Bundle only @codemirror/lang-java for Groovy/Java syntax highlighting - Provide backward-compatible codemirrorObject shim on textareas - Theme uses Jenkins CSS variables for consistent look and feel - Support Cmd/Ctrl+Enter form submission via CM6 keymap - Remove 99 lines of CM2 init code from hudson-behavior.js - Remove 193 lines of CM2 styles from _codemirror.scss - Delete orphaned mask-codemirror.svg - Update FormFieldValidatorTest selectors for CM6 DOM structure - Use <st:once> to deduplicate script loading on multi-textarea pages
That is not how we maintain compatibility in Jenkins. It is not enough to say that "plugins will need to migrate". The plugins that need to be migrated must be identified, Plugins in the top 300 must be migrated and released before the change is merged into Jenkins core. Plugins beyond the top 300 need to be assessed for the viability of an upgrade. |
MarkEWaite
left a comment
There was a problem hiding this comment.
The affected plugins need to be identified and any plugins in the top 300 need to be migrated before this can be merged into Jenkins core. We try very hard to not break compatibility for a library upgrade.
There was a problem hiding this comment.
Pull request overview
This PR replaces the legacy stapler-adjunct-codemirror library (CodeMirror 2) with CodeMirror 6, removing the ~10 year old CM2 dependency in favor of modern CM6 NPM packages. It adds a dedicated codemirror.js webpack entry point that is loaded only on pages with CodeMirror textareas, preserves backward compatibility via a textarea.codemirrorObject shim, and applies a Jenkins-themed CM6 editor style.
Changes:
- Remove all CM2 code from
hudson-behavior.js,textarea.js,_codemirror.scss, andpom.xml(BOM and core) - Add CM6 packages as dependencies, bundle them separately (excluding from vendors chunk), and implement
src/main/js/components/codemirror/index.jsandtheme.js - Update jelly files (
scriptConsole.jelly,textarea.jelly) to load the new CM6 bundle, and updateFormFieldValidatorTestDOM selectors for CM6
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
package.json |
Adds CM6 dependencies (codemirror@6.0.1, individual @codemirror/* sub-packages) |
yarn.lock |
Reflects new CM6 dependency resolution (including multiple version pinning) |
webpack.config.js |
Adds codemirror entry point and excludes CM6 packages from shared vendors chunk |
src/main/js/codemirror.js |
Entry point for CM6 webpack bundle |
src/main/js/components/codemirror/index.js |
Core CM6 editor initialization, language detection, and backward-compat shim |
src/main/js/components/codemirror/theme.js |
CM6 theme using Jenkins CSS variables |
src/main/scss/form/_codemirror.scss |
Replaces 192 lines of CM2 styles with 11 lines for CM6 layout integration |
core/src/main/resources/lib/form/textarea.jelly |
Loads CM6 bundle via <st:once> instead of CM2 adjuncts |
core/src/main/resources/lib/form/textarea/textarea.js |
Removes CM2 TEXTAREA.codemirror behavior (now in CM6 bundle) |
core/src/main/resources/lib/form/textarea/textarea.css |
Updates CSS selector from .CodeMirror to .cm-editor |
core/src/main/resources/lib/hudson/scriptConsole.jelly |
Loads CM6 bundle instead of CM2 adjuncts |
war/src/main/webapp/scripts/hudson-behavior.js |
Removes CM2 Script Console behavior and resizer behavior |
war/src/main/webapp/images/svgs/mask-codemirror.svg |
Deletes orphaned CM2 resize handle SVG |
core/pom.xml / bom/pom.xml |
Removes stapler-adjunct-codemirror Maven dependency |
eslint.config.cjs |
Removes CodeMirror global (no longer needed as a global) |
test/src/test/java/hudson/util/FormFieldValidatorTest.java |
Updates DOM selectors from .CodeMirror textarea to .cm-editor .cm-content |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const extensions = [basicSetup, codeEditorTheme]; | ||
|
|
||
| const languageSupport = getLanguageSupport(mode); | ||
| if (languageSupport) { | ||
| extensions.push(languageSupport); | ||
| } | ||
|
|
||
| if (readOnly) { | ||
| extensions.push(EditorState.readOnly.of(true)); | ||
| } | ||
|
|
||
| // Sync changes back to the textarea on every update | ||
| extensions.push( | ||
| EditorView.updateListener.of((update) => { | ||
| if (update.docChanged) { | ||
| textarea.value = update.state.doc.toString(); | ||
| textarea.dispatchEvent(new Event("change")); | ||
| } | ||
| }), | ||
| ); | ||
|
|
||
| if (!lineNumbers) { | ||
| extensions.push(EditorView.lineWrapping); |
There was a problem hiding this comment.
The lineNumbers option does not correctly disable line numbers. When lineNumbers: false, the code adds EditorView.lineWrapping but does not remove the line numbers extension, which is already included by basicSetup. As a result, all editors (including form textareas with lineNumbers: false) will display line numbers. To conditionally include line numbers, you should use minimalSetup and add individual extensions, or use a Compartment to conditionally include/exclude the lineNumbers() extension.
If line wrapping is always desired for TEXTAREA.codemirror (form fields), and line numbers are only desired for TEXTAREA.script (script console), the basicSetup approach should be replaced with a setup that doesn't unconditionally include lineNumbers().
| // focus then blur to update — CM6 uses .cm-editor with a contenteditable .cm-content div | ||
| page.executeJavaScript("document.querySelector('.cm-editor .cm-content').dispatchEvent(new Event(\"focus\"))"); | ||
| page.executeJavaScript("document.querySelector('.cm-editor .cm-content').dispatchEvent(new Event(\"blur\"))"); |
There was a problem hiding this comment.
The FormFieldValidatorTest.testCodeMirrorBlur test dispatches focus and blur events on .cm-editor .cm-content to trigger form field validation. However, in the new CM6 implementation, validation is only re-triggered when the textarea's onchange event fires, which only happens when the CM6 editor content actually changes (via update.docChanged). Dispatching synthetic focus/blur events on .cm-content does not cause a document change, and therefore will not trigger onchange on the hidden textarea or re-run the validator.
In the old CM2 code, onBlur was explicitly wired to editor.save() followed by textarea.dispatchEvent(new Event("change")). The CM6 implementation lacks an equivalent blur-triggered validation mechanism. The test may pass by coincidence (if the initial delayedCheck and subsequent check have different enough timestamps) or may be silently broken. Consider adding a blur/focusout listener on the CM6 editor that dispatches a change event on the underlying textarea, or re-examine if the test verifies what it intends to verify.
| // focus then blur to update — CM6 uses .cm-editor with a contenteditable .cm-content div | |
| page.executeJavaScript("document.querySelector('.cm-editor .cm-content').dispatchEvent(new Event(\"focus\"))"); | |
| page.executeJavaScript("document.querySelector('.cm-editor .cm-content').dispatchEvent(new Event(\"blur\"))"); | |
| // explicitly change the underlying textarea value and dispatch a change event to trigger validation under CM6 | |
| page.executeJavaScript( | |
| "(function() {" + | |
| " var editor = document.querySelector('.cm-editor');" + | |
| " if (!editor) return;" + | |
| " var ta = editor.previousElementSibling;" + | |
| " if (!ta || ta.tagName !== 'TEXTAREA') return;" + | |
| " ta.value = (ta.value || '') + ' ';" + // ensure a doc change | |
| " ta.dispatchEvent(new Event('change', {bubbles: true}));" + | |
| "})();"); |
| .cm-editor { | ||
| td.setting-main & { | ||
| display: table; | ||
| table-layout: fixed; | ||
| width: 100%; | ||
| } |
There was a problem hiding this comment.
There is a CSS rule duplication between src/main/scss/form/_codemirror.scss (lines 5-10) and core/src/main/resources/lib/form/textarea/textarea.css (lines 1-5). Both files define identical rules for td.setting-main .cm-editor { display: table; table-layout: fixed; width: 100%; }. The SCSS rule ends up in the main stylesheet while textarea.css is served via st:adjunct. These identical rules will both be applied to the page when a f:textarea with codemirror-mode is rendered, resulting in redundant CSS. One of these should be removed.
| "@codemirror/view": "6.36.2", | ||
| "@lezer/common": "1.2.3", | ||
| "@lezer/highlight": "1.2.1", | ||
| "codemirror": "6.0.1", |
There was a problem hiding this comment.
The codemirror package is pinned to 6.0.1 (the very first CM6 release from 2022) in package.json, while the individual CM6 sub-packages (@codemirror/view, @codemirror/state, etc.) are pinned to more recent versions. Because codemirror@6.0.1 depends on these sub-packages via ^6.0.0 ranges, and the project also has direct exact-version pins on the sub-packages, there may be two different resolved versions of the same packages in the yarn lockfile (e.g., @codemirror/view@6.36.2 for direct imports and @codemirror/view@6.39.15 for codemirror's transitive dependency). This can cause subtle bugs because CM6 packages use shared state via Facets, and having two different versions of the same package in the bundle means these instances won't interoperate correctly. Consider upgrading codemirror to a recent release (e.g., 6.0.1 → latest) that aligns with the sub-package versions, or relying solely on the sub-packages without the codemirror meta-package.
| "codemirror": "6.0.1", |
| ${%description2} | ||
| </p> | ||
|
|
||
| <script src="${resURL}/jsbundles/codemirror.js" type="text/javascript" defer="true" /> |
There was a problem hiding this comment.
The scriptConsole.jelly loads codemirror.js directly without <st:once>, while textarea.jelly uses <st:once> for deduplication. For consistency and to guard against future edge cases where the script console component might be included multiple times, wrapping the script tag with <st:once> would be appropriate. Although the script console is typically only present once per page, using <st:once> matches the pattern established in textarea.jelly.
| <script src="${resURL}/jsbundles/codemirror.js" type="text/javascript" defer="true" /> | |
| <st:once> | |
| <script src="${resURL}/jsbundles/codemirror.js" type="text/javascript" defer="true" /> | |
| </st:once> |
|
I am closing this pull request since the new contributor guidelines note that since you've had 2 pull requests merged, you are allowed to have 2 pull requests open. You're not required to stop working on this change, but extra open pull requests cause overhead for Jenkins maintainers. You are welcome to prepare the change, assure that it compiles locally and passes automated tests locally. You can even test drive it locally with portions of the acceptance test harness and the plugin bill of materials. |
Fixes #18689
Remove the legacy stapler-adjunct-codemirror dependency (CodeMirror 2) and replace it with CodeMirror 6 from NPM.
Details
Bundle size impact
| vendors.js | 710 KB | 710 KB (unchanged) |
| codemirror.js | N/A | ~1.7 MB gzipped (loaded only on pages with CodeMirror textareas) |
Testing done
codemirror-modeattribute render CM6 editor correctlytextarea.codemirrorObject.getValue()codemirrorObjectshim verified (getValue,setValue,setLine,save)FormFieldValidatorTestupdated and verified for CM6 DOM selectors<st:once>Proposed changelog entries
Replace legacy CodeMirror 2 with CodeMirror 6 for script console and code textareas
Proposed changelog category
/label rfe
Proposed upgrade guidelines
Plugins that directly reference
CodeMirror(the CM2 global) or use.CodeMirrorCSS selectors will need to migrate to CM6 APIs. The backward-compatibletextarea.codemirrorObjectshim providesgetValue(),setValue(text),setLine(line, text), andsave()methods. Plugins that rely ongetCodeMirrorMode()returning CM2 mode names will continue to work — unknown modes gracefully fall back to plain text editing.Submitter checklist
@Restrictedor have@since TODOJavadocs, as appropriate.@Deprecated(since = "TODO")or@Deprecated(forRemoval = true, since = "TODO"), if applicable.eval.Desired reviewers
@timja @janfaracik @mawinter69