Replace Ace Editor with Monaco Editor #801
Conversation
- Add most configuration options - Dispose and create editor depending on whether in view - Fix sizing issue
bennothommo
added
enhancement
This comment was marked as outdated.
This comment was marked as outdated.
modules/backend/controllers/preferences/_field_editor_preview.php
Outdated
Show resolved
Hide resolved
…wip/monaco-editor
… events for editor
# Conflicts: # modules/backend/formwidgets/codeeditor/assets/css/codeeditor.css # modules/system/assets/ui/storm.css
…as well as codeeditor
Also improved the UX for previewing the code editor settings, added one-dark-pro and winter themes, renamed codeeditor's source JS file to codeeditor.js, fixed a bug with alt key binding, and removed unused code from the Preferences controller.
RichEditor is currently using Froala v2.9.3 which is difficult to customize as we don't have the original DRM files anymore. Ideally we would like to replace the codeeditor view of Froala with Monaco, but for now we'll retain ace for this use case.
…less/markdowneditor.less
…s/_markdowneditor.php
Markdown editor needs some more work, we might look at using TipTap or Milkdown / Crepe in the future.
Interacting with the PackageManager triggers a need to use the database connection to get the active theme, the database connection is not available until the boot process.
…educe issues caused by timing issues with webpack loading
For anyone else trying to track down self being clobbered, this is very handy:
(function () {
try {
const desc = Object.getOwnPropertyDescriptor(window, 'self');
console.log('window.self descriptor:', desc);
let _selfValue = window;
Object.defineProperty(window, 'self', {
configurable: true,
get() { return _selfValue; },
set(v) {
console.error('window.self set to:', v);
console.trace('stack:'); // <- prints the exact script/line
_selfValue = v;
},
});
} catch (e) {
console.warn('Failed to trap window.self:', e);
}
})();
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
♻️ Duplicate comments (3)
modules/backend/formwidgets/codeeditor/assets/js/codeeditor.js (3)
196-212: Click listener cleanup missing indispose()— issue persists from prior review.The click listener is added each time
attachListeners()runs (viacreateEditor()), but cleanup only happens indestruct(), notdispose(). SinceIntersectionObservertriggersdispose()/createEditor()cycles, multiple click listeners can accumulate.🔎 Proposed fix
dispose() { if (this.disposables.length > 0) { this.disposables.forEach((disposable) => { disposable.dispose(); }); this.disposables = []; } if (this.resizeListener) { window.removeEventListener('resize', this.callbacks.resize); this.resizeListener = false; } + if (this.clickListener) { + document.removeEventListener('click', this.callbacks.click, { capture: true }); + this.clickListener = false; + } if (this.editor) { this.editor.dispose(); this.editor = null; } this.events.fire('dispose', this, this.editor); }And in
attachListeners()(around line 401-403):document.addEventListener('click', this.callbacks.click, { capture: true, }); + this.clickListener = true;
1497-1508: Null pointer dereference whenresolveris not provided — issue persists.When
addCodeLensis called without a resolver (using the defaultnull), line 1504 throws a TypeError becauseresolver(...)is invoked before the nullish coalescing operator evaluates.🔎 Proposed fix
addCodeLens(language, provider, resolver = null) { this.disposables.push( monaco.languages.registerCodeLensProvider(language, { provideCodeLenses: (model, token) => { return provider(model, token); }, resolveCodeLens: (model, codeLens, token) => { - return resolver(model, codeLens, token) ?? codeLens; + return resolver ? resolver(model, codeLens, token) : codeLens; }, }) ); }
401-411: MissingclickListenerflag update.The
clickListenerproperty is initialized but never set totruewhen the listener is added, making the flag useless for tracking.🔎 Proposed fix
document.addEventListener('click', this.callbacks.click, { capture: true, }); + this.clickListener = true;
🧹 Nitpick comments (8)
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_less_less_js.js (1)
1-187: Auto-generated Monaco Editor language bundle — LGTM with one consideration.This is a webpack-generated chunk from the official
monaco-editorpackage (v0.34.1) providing LESS language support. The tokenizer and configuration are maintained upstream by Microsoft.One optional consideration: the inline base64 source map on line 187 increases bundle size. If source maps aren't needed for debugging Monaco's internal language definitions in production, consider configuring webpack to externalize or omit source maps for these vendor chunks.
modules/backend/assets/less/layout/fancylayout.less (1)
440-442: Consider using a LESS variable instead of hardcoded color.The hardcoded
#000000value reduces maintainability. For consistency with the rest of the file (which uses variables like@color-fancy-master-tabs-active-text), consider defining a variable for this color value.🔎 Suggested refactor
Define a variable (e.g.,
@color-fancy-secondary-tabs-icon-default) and use it here:.tab-collapse-icon.primary { - color: #000000; + color: @color-fancy-secondary-tabs-icon-default; }modules/backend/controllers/Preferences.php (1)
4-11: Imports look good, butApplicationExceptionandFileappear unused.The explicit facade imports are appropriate. However,
ApplicationException(line 9) andFile(line 10) are imported but not used in the visible controller code.🔎 Consider removing unused imports
use Backend\Facades\Backend; use Backend\Facades\BackendMenu; use Backend\Models\Preference as PreferenceModel; use Illuminate\Support\Facades\Lang; use System\Classes\SettingsManager; -use Winter\Storm\Exception\ApplicationException; -use Winter\Storm\Support\Facades\File; use Winter\Storm\Support\Facades\Flash;modules/backend/assets/vendor/ace-codeeditor/codeeditor.js (1)
247-263: Variable redeclaration in nested loops.The static analysis correctly identifies that
iandlenare redeclared in the second loop (line 255) after being declared in the first loop (line 251). While JavaScript hoistsvardeclarations and this works at runtime, it's a code smell that can cause confusion.🔎 Proposed fix to eliminate redeclaration
CodeEditor.prototype.disposeAttachedControls = function() { this.editor.destroy() var keys = Object.keys(this.editor.renderer) - for (var i=0, len=keys.length; i<len; i++) + for (var i=0, len=keys.length; i<len; i++) { this.editor.renderer[keys[i]] = null + } keys = Object.keys(this.editor) - for (var i=0, len=keys.length; i<len; i++) + for (i=0, len=keys.length; i<len; i++) { this.editor[keys[i]] = null + } this.editor = nullmodules/backend/formwidgets/codeeditor/assets/js/codeeditor.js (4)
28-35: Consider revoking blob URLs to prevent memory leaks.Blob URLs created via
URL.createObjectURLshould ideally be revoked after use. However, since these worker URLs are typically loaded once and cached by the browser, the impact is minimal.🔎 Proposed improvement (if workers are loaded multiple times)
// Cross-origin: create blob URL wrapper const script = `importScripts('${workerUrl}');`; const blob = new Blob([script], { type: 'application/javascript' }); - return URL.createObjectURL(blob); + const blobUrl = URL.createObjectURL(blob); + // Note: Cannot revoke immediately as worker needs the URL + // Consider caching blob URLs if workers are recreated frequently + return blobUrl;
435-446: Consider null checks for model access.
getValue()andsetValue()accessthis.modeldirectly. If called when the editor is disposed (e.g., element out of viewport), this will throw.🔎 Proposed defensive check
getValue() { + if (!this.model) { + return this.valueBag?.value ?? ''; + } return this.model.getValue(); } setValue(value) { + if (!this.model) { + if (this.valueBag) { + this.valueBag.value = value; + } + return; + } this.model.setValue(value); }
1029-1039: Early return added — redundant regex match remains.The past review issue is fixed with the early return at line 1034. However, the regex is matched twice (lines 1032 and 1036), which is inefficient.
🔎 Proposed optimization
if (color.length === 9) { return color; } - if (!color.match(/^#(..)(..)(..)(..)$/)) { + const rgbaMatch = color.match(/^#(..)(..)(..)(..)$/); + if (!rgbaMatch) { // Invalid color format - return original color return color; } - const rgba = color.match(/^#(..)(..)(..)(..)$/).slice(1).map((c) => parseInt(c, 16)); + const rgba = rgbaMatch.slice(1).map((c) => parseInt(c, 16)); rgba[3] = (rgba[3] / 0xFF).toPrecision(2); return `rgba(${rgba.join(', ')})`;
1047-1056: Missing null checks for regex matches could cause runtime errors.Both regex matches on lines 1052 and 1055 could return
nullif the color format is unexpected, causing.slice(1)to throw.🔎 Proposed defensive fix
rgbColor(color) { if (typeof color === 'object') { return color; } if (color[0] === '#') { - return color.match(/^#(..)(..)(..)/).slice(1).map((c) => parseInt(c, 16)); + const match = color.match(/^#(..)(..)(..)$/); + return match ? match.slice(1).map((c) => parseInt(c, 16)) : [0, 0, 0]; } - - return color.match(/\(([^,]+),([^,]+),([^,]+)/).slice(1).map((c) => parseInt(c, 10)); + const match = color.match(/\(([^,]+),\s*([^,]+),\s*([^,)]+)/); + return match ? match.slice(1).map((c) => parseInt(c, 10)) : [0, 0, 0]; }
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
modules/backend/formwidgets/colorpicker/assets/js/dist/colorpicker.jsis excluded by!**/dist/**modules/backend/formwidgets/iconpicker/assets/js/dist/iconpicker.jsis excluded by!**/dist/**modules/backend/formwidgets/sensitive/assets/js/dist/sensitive.jsis excluded by!**/dist/**
📒 Files selected for processing (135)
modules/backend/ServiceProvider.phpmodules/backend/assets/.gitignoremodules/backend/assets/css/winter.cssmodules/backend/assets/js/preferences/preferences.jsmodules/backend/assets/less/layout/fancylayout.lessmodules/backend/assets/ui/js/build/backend.jsmodules/backend/assets/ui/js/build/manifest.jsmodules/backend/assets/ui/js/build/vendor.jsmodules/backend/assets/vendor/ace-codeeditor/build-min.jsmodules/backend/assets/vendor/ace-codeeditor/build.jsmodules/backend/assets/vendor/ace-codeeditor/codeeditor.jsmodules/backend/assets/vendor/ace/ace.jsmodules/backend/assets/vendor/ace/ext-emmet.jsmodules/backend/assets/vendor/ace/ext-language_tools.jsmodules/backend/assets/vendor/ace/ext-searchbox.jsmodules/backend/assets/vendor/ace/mode-css.jsmodules/backend/assets/vendor/ace/mode-html.jsmodules/backend/assets/vendor/ace/mode-javascript.jsmodules/backend/assets/vendor/ace/mode-less.jsmodules/backend/assets/vendor/ace/mode-markdown.jsmodules/backend/assets/vendor/ace/mode-php.jsmodules/backend/assets/vendor/ace/mode-plain_text.jsmodules/backend/assets/vendor/ace/mode-sass.jsmodules/backend/assets/vendor/ace/mode-scss.jsmodules/backend/assets/vendor/ace/mode-twig.jsmodules/backend/assets/vendor/ace/mode-yaml.jsmodules/backend/assets/vendor/ace/snippets/css.jsmodules/backend/assets/vendor/ace/snippets/html.jsmodules/backend/assets/vendor/ace/snippets/javascript.jsmodules/backend/assets/vendor/ace/snippets/markdown.jsmodules/backend/assets/vendor/ace/snippets/php-inline.jsmodules/backend/assets/vendor/ace/snippets/php.jsmodules/backend/assets/vendor/ace/snippets/plain_text.jsmodules/backend/assets/vendor/ace/snippets/sass.jsmodules/backend/assets/vendor/ace/snippets/scss.jsmodules/backend/assets/vendor/ace/snippets/text.jsmodules/backend/assets/vendor/ace/snippets/twig.jsmodules/backend/assets/vendor/ace/snippets/yaml.jsmodules/backend/assets/vendor/ace/theme-ambiance.jsmodules/backend/assets/vendor/ace/theme-chaos.jsmodules/backend/assets/vendor/ace/theme-chrome.jsmodules/backend/assets/vendor/ace/theme-clouds.jsmodules/backend/assets/vendor/ace/theme-clouds_midnight.jsmodules/backend/assets/vendor/ace/theme-cobalt.jsmodules/backend/assets/vendor/ace/theme-crimson_editor.jsmodules/backend/assets/vendor/ace/theme-dawn.jsmodules/backend/assets/vendor/ace/theme-dreamweaver.jsmodules/backend/assets/vendor/ace/theme-eclipse.jsmodules/backend/assets/vendor/ace/theme-github.jsmodules/backend/assets/vendor/ace/theme-idle_fingers.jsmodules/backend/assets/vendor/ace/theme-iplastic.jsmodules/backend/assets/vendor/ace/theme-katzenmilch.jsmodules/backend/assets/vendor/ace/theme-kr_theme.jsmodules/backend/assets/vendor/ace/theme-kuroir.jsmodules/backend/assets/vendor/ace/theme-merbivore.jsmodules/backend/assets/vendor/ace/theme-merbivore_soft.jsmodules/backend/assets/vendor/ace/theme-mono_industrial.jsmodules/backend/assets/vendor/ace/theme-monokai.jsmodules/backend/assets/vendor/ace/theme-pastel_on_dark.jsmodules/backend/assets/vendor/ace/theme-solarized_dark.jsmodules/backend/assets/vendor/ace/theme-solarized_light.jsmodules/backend/assets/vendor/ace/theme-sqlserver.jsmodules/backend/assets/vendor/ace/theme-terminal.jsmodules/backend/assets/vendor/ace/theme-textmate.jsmodules/backend/assets/vendor/ace/theme-tomorrow.jsmodules/backend/assets/vendor/ace/theme-tomorrow_night.jsmodules/backend/assets/vendor/ace/theme-tomorrow_night_blue.jsmodules/backend/assets/vendor/ace/theme-tomorrow_night_bright.jsmodules/backend/assets/vendor/ace/theme-tomorrow_night_eighties.jsmodules/backend/assets/vendor/ace/theme-twilight.jsmodules/backend/assets/vendor/ace/theme-vibrant_ink.jsmodules/backend/assets/vendor/ace/theme-xcode.jsmodules/backend/assets/vendor/ace/worker-css.jsmodules/backend/assets/vendor/ace/worker-html.jsmodules/backend/assets/vendor/ace/worker-javascript.jsmodules/backend/assets/vendor/ace/worker-php.jsmodules/backend/assets/vendor/emmet/emmet.jsmodules/backend/controllers/Preferences.phpmodules/backend/controllers/preferences/_field_editor_preview_lang.phpmodules/backend/formwidgets/CodeEditor.phpmodules/backend/formwidgets/MarkdownEditor.phpmodules/backend/formwidgets/RichEditor.phpmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/133.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/257.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/39.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/851.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_css_css_js.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_html_html_js.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_ini_ini_js.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_javascript_javascript_js.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_less_less_js.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_markdown_markdown_js.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_mysql_mysql_js.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_php_php_js.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_scss_scss_js.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_twig_twig_js.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_typescript_typescript_js.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_xml_xml_js.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_yaml_yaml_js.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_language_css_cssMode_js.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_language_html_htmlMode_js.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_language_json_jsonMode_js.jsmodules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_language_typescript_tsMode_js.jsmodules/backend/formwidgets/codeeditor/assets/js/build/css.worker.jsmodules/backend/formwidgets/codeeditor/assets/js/build/editor.worker.jsmodules/backend/formwidgets/codeeditor/assets/js/build/html.worker.jsmodules/backend/formwidgets/codeeditor/assets/js/build/json.worker.jsmodules/backend/formwidgets/codeeditor/assets/js/build/ts.worker.jsmodules/backend/formwidgets/codeeditor/assets/js/codeeditor.jsmodules/backend/formwidgets/codeeditor/assets/themes/one-dark-pro.jsonmodules/backend/formwidgets/codeeditor/assets/themes/winter.jsonmodules/backend/formwidgets/codeeditor/assets/winter.mix.jsmodules/backend/formwidgets/markdowneditor/partials/_markdowneditor.phpmodules/backend/formwidgets/richeditor/partials/_richeditor.phpmodules/backend/lang/en/lang.phpmodules/backend/layouts/_head.phpmodules/backend/layouts/auth.phpmodules/backend/models/Preference.phpmodules/backend/models/preference/fields.yamlmodules/backend/widgets/table/assets/js/build-min.jsmodules/backend/widgets/table/assets/js/table.processor.dropdown.jsmodules/backend/winter.mix.jsmodules/cms/ServiceProvider.phpmodules/cms/controllers/Index.phpmodules/system/ServiceProvider.phpmodules/system/assets/js/build/manifest.jsmodules/system/assets/js/build/system.debug.jsmodules/system/assets/js/build/system.jsmodules/system/assets/js/snowboard/build/snowboard.base.debug.jsmodules/system/assets/js/snowboard/build/snowboard.base.jsmodules/system/assets/js/snowboard/build/snowboard.data-attr.jsmodules/system/assets/js/snowboard/build/snowboard.extras.jsmodules/system/assets/js/snowboard/build/snowboard.request.jspackage.json
✅ Files skipped from review due to trivial changes (3)
- modules/backend/assets/vendor/ace-codeeditor/build.js
- modules/backend/formwidgets/codeeditor/assets/themes/one-dark-pro.json
- modules/backend/assets/.gitignore
🚧 Files skipped from review as they are similar to previous changes (2)
- modules/backend/assets/ui/js/build/backend.js
- modules/backend/controllers/preferences/_field_editor_preview_lang.php
🧰 Additional context used
🧬 Code graph analysis (7)
modules/backend/formwidgets/MarkdownEditor.php (2)
modules/backend/facades/BackendAuth.php (1)
BackendAuth(16-27)modules/system/traits/AssetMaker.php (1)
addJs(122-149)
modules/backend/formwidgets/RichEditor.php (2)
modules/backend/facades/BackendAuth.php (1)
BackendAuth(16-27)modules/system/traits/AssetMaker.php (1)
addJs(122-149)
modules/backend/assets/vendor/ace-codeeditor/codeeditor.js (3)
modules/backend/formwidgets/CodeEditor.php (1)
CodeEditor(15-271)modules/backend/classes/FormField.php (1)
options(228-246)modules/cms/assets/js/winter.cmspage.js (1)
$form(268-268)
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/133.js (1)
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/39.js (21)
t(7-7)t(7-7)t(7-7)t(7-7)n(7-7)n(7-7)n(7-7)r(7-7)r(7-7)r(7-7)r(7-7)r(7-7)r(7-7)r(7-7)r(7-7)i(7-7)i(7-7)i(7-7)i(7-7)i(7-7)a(7-7)
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/39.js (2)
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/133.js (23)
t(7-7)t(7-7)t(7-7)t(7-7)t(7-7)t(7-7)n(7-7)n(7-7)n(7-7)r(7-7)r(7-7)r(7-7)r(7-7)r(7-7)r(7-7)r(7-7)r(7-7)r(7-7)i(7-7)i(7-7)i(7-7)i(7-7)i(7-7)modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/257.js (20)
t(7-7)t(7-7)t(7-7)t(7-7)t(7-7)n(7-7)n(7-7)n(7-7)r(7-7)r(7-7)r(7-7)r(7-7)r(7-7)r(7-7)r(7-7)i(7-7)i(7-7)i(7-7)i(7-7)i(7-7)
modules/backend/ServiceProvider.php (1)
modules/system/classes/asset/PackageManager.php (2)
PackageManager(24-414)registerCallback(185-188)
modules/backend/controllers/Preferences.php (3)
modules/backend/facades/BackendMenu.php (1)
BackendMenu(30-41)modules/backend/models/Preference.php (1)
Preference(21-323)modules/system/classes/SettingsManager.php (1)
SettingsManager(14-417)
🪛 Biome (2.1.2)
modules/backend/assets/ui/js/build/manifest.js
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
modules/backend/assets/vendor/ace-codeeditor/codeeditor.js
[error] 255-255: Shouldn't redeclare 'i'. Consider to delete it or rename it.
'i' is defined here:
(lint/suspicious/noRedeclare)
[error] 255-255: Shouldn't redeclare 'len'. Consider to delete it or rename it.
'len' is defined here:
(lint/suspicious/noRedeclare)
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/133.js
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/39.js
[error] 7-7: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 7-7: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 7-7: This case is falling through to the next case.
Add a break or return statement to the end of this case to prevent fallthrough.
(lint/suspicious/noFallthroughSwitchClause)
[error] 7-7: Other switch clauses can erroneously access this declaration.
Wrap the declaration in a block to restrict its access to the switch clause.
The declaration is defined in this switch clause:
Safe fix: Wrap the declaration in a block.
(lint/correctness/noSwitchDeclarations)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/257.js
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 7-7: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
modules/backend/assets/js/preferences/preferences.js
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
🔇 Additional comments (37)
modules/backend/assets/ui/js/build/manifest.js (1)
1-1: This file is a committed build artifact—build artifacts are intentionally tracked in Winter CMS.The functional change adding chunk 908 tracking for the Monaco editor is correct and aligns with Winter CMS's established practice of committing compiled build artifacts to version control. No changes are needed.
Note: Static analysis warnings about variables being used before declaration are false positives typical of minified webpack bundles and can be safely ignored.
Likely an incorrect or invalid review comment.
modules/backend/assets/css/winter.css (1)
1024-1025: Compiled CSS output looks correct.These changes introduce explicit color handling for the tab collapse icon in both collapsed and non-collapsed states, which improves visual distinction. The compiled output correctly reflects the source LESS changes.
Note: Since this is compiled CSS, the substantive review should focus on the LESS source file (fancylayout.less).
modules/backend/assets/less/layout/fancylayout.less (1)
444-447: Good improvement using a LESS variable.Changing from hardcoded
whiteto@color-fancy-master-tabs-active-textimproves maintainability and consistency with the rest of the codebase. This makes it easier to update the color scheme in the future.modules/backend/formwidgets/MarkdownEditor.php (2)
1-11: LGTM! Explicit facade imports and namespace updates.The imports are now using fully-qualified Winter\Storm facades (
Html,Markdown) andBackend\Facades\BackendAuth, which aligns with the codebase conventions shown in the relevant code snippets.
92-97: Intentional retention of ACE for MarkdownEditor.The asset path update to
/modules/backend/assets/vendor/ace-codeeditor/build-min.jscorrectly keeps ACE for the Markdown editor while Monaco is adopted elsewhere. This aligns with the PR discussion noting that Monaco integration for Markdown is deferred.modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_ini_ini_js.js (1)
1-88: LGTM! Standard Monaco INI language bundle.This is a webpack-generated Monaco Editor basic language module for INI files. The bundle correctly exports
confandlanguagebindings following the established Monaco language definition pattern used across other language bundles in this PR.modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_mysql_mysql_js.js (1)
1-895: LGTM! Standard Monaco MySQL language bundle.This is a webpack-generated Monaco Editor basic language module for MySQL. The comprehensive SQL syntax support including keywords, operators, built-in functions, and tokenizer states follows the standard Monaco language definition pattern.
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_scss_scss_js.js (1)
1-262: LGTM! Standard Monaco SCSS language bundle.This is a webpack-generated Monaco Editor basic language module for SCSS. The tokenizer correctly handles SCSS-specific constructs including variables, mixins, functions, control statements, and nested properties.
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_php_php_js.js (1)
1-491: LGTM! Standard Monaco PHP language bundle.This is a webpack-generated Monaco Editor basic language module for PHP with comprehensive support for mixed PHP/HTML content. The tokenizer correctly handles embedded contexts (JavaScript in
<script>, CSS in<style>) and PHP block transitions, which is essential for Winter CMS template editing.modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_twig_twig_js.js (1)
1-343: LGTM! Standard Monaco Twig language bundle.This is a webpack-generated Monaco Editor basic language module for Twig. The tokenizer correctly handles Twig-specific delimiters (
{# #},{% %},{{ }}), verbatim blocks, and embedded HTML/JavaScript/CSS contexts. This is particularly valuable for Winter CMS template editing.modules/backend/controllers/Preferences.php (1)
34-42: LGTM! Asset loading simplified for preferences.The constructor correctly removes direct code editor asset loading. The code editor assets are now managed centrally through the ServiceProvider's asset bundle registration, and only the preferences-specific JavaScript remains.
modules/backend/formwidgets/RichEditor.php (1)
1-14: LGTM! Import additions are all used.All newly added facade imports (Backend, BackendAuth, App, Lang, Request, Config, Event, File) are verified as used within the file, which improves code clarity and follows best practices.
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_css_css_js.js (1)
1-204: Vendor bundle looks correct.This is an auto-generated webpack bundle containing the standard Monaco Editor CSS language definition (v0.34.1). The module correctly exports
confandlanguagefor CSS syntax highlighting support.modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_xml_xml_js.js (1)
1-127: Vendor bundle looks correct.Standard Monaco Editor XML language definition bundle with proper tokenizer states and editor configuration.
modules/backend/ServiceProvider.php (3)
94-94: Ace editor bundle retained for backward compatibility.Adding the
ace-codeeditorbundle aligns with the PR description stating that Ace is being retained for RichEditor and MarkdownEditor while Monaco is introduced for the primary code editor.
114-117: Package registration follows established naming convention.The file exists at the specified path, and the registration pattern using
PackageManager::instance()->registerCallbackis consistent with other callback registrations in this file (MailManager, CombineAssets, BackendMenu, BackendAuth, and SettingsManager).
56-57: Asset bundle registration in boot() method is correct.
registerAssetBundles()is appropriately placed in theboot()method where all asset bundle callbacks are registered. This is consistent with the pattern in the cms and system modules and aligns with Laravel's service provider lifecycle, whereboot()executes after all services are registered.modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_yaml_yaml_js.js (1)
1-216: Vendor bundle looks correct.Standard Monaco Editor YAML language definition with comprehensive tokenization support for YAML-specific constructs including anchors, tags, and flow collections.
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_markdown_markdown_js.js (1)
1-222: Vendor bundle looks correct.Standard Monaco Editor Markdown language definition with support for tables, fenced code blocks, and embedded HTML/CSS/JavaScript content.
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_html_html_js.js (1)
1-314: Vendor bundle looks correct.Standard Monaco Editor HTML language definition with comprehensive support for embedded script/style content, proper handling of empty elements, and type-aware language embedding.
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_javascript_javascript_js.js (1)
1-449: Vendor bundle looks correct.Standard Monaco Editor JavaScript/TypeScript language definitions. The JavaScript module appropriately extends the TypeScript configuration while maintaining its own keyword list without TypeScript-specific keywords.
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/node_modules_monaco-editor_esm_vs_basic-languages_typescript_typescript_js.js (1)
1-359: Vendor bundle - LGTM.This is an auto-generated webpack bundle for Monaco Editor's TypeScript language support (v0.34.1). The static analysis warnings about "variable used before declaration" are false positives caused by webpack's bundling patterns and the module re-export mechanism on line 39. These patterns are valid at runtime.
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/133.js (1)
1-2: Minified vendor bundle - LGTM.This is a minified Monaco Editor HTML language service integration bundle. The static analysis warnings on line 7 are false positives caused by webpack's minification and module bundling patterns. The code correctly implements WorkerManager with idle timeout handling and comprehensive language service adapters.
modules/backend/formwidgets/CodeEditor.php (2)
250-270: Preference migration logic handles backwards compatibility well.The fallback logic
$preferences->editor_enable_folding ?? ($preferences->editor_code_folding !== 'manual')gracefully handles migration from the old string-basededitor_code_foldingpreference to the new booleaneditor_enable_folding.The new preference mappings (
showMinimap,bracketColors,showColors) follow the established pattern.
240-244: Asset bundle path updated correctly.The change from
js/build-min.jstojs/build/codeeditor.bundle.jsaligns with the Monaco Editor integration. Bundle file exists at the expected path, along with associated chunk files and CSS assets.modules/backend/assets/vendor/ace-codeeditor/codeeditor.js (2)
446-448: Data control attribute correctly differentiated.The initialization now uses
data-control="ace-codeeditor"which properly distinguishes this ACE-based editor from the new Monaco-baseddata-control="codeeditor". This allows both editors to coexist during the transition period.
453-466: Emmet twig syntax fix retained.The Emmet extension fix that normalizes
twigsyntax tohtmlis preserved, ensuring backwards compatibility with existing Twig template editing workflows.modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/39.js (1)
1-7: Minified vendor bundle - LGTM.This is a minified Monaco Editor JSON language service bundle. The static analysis warnings (switch fallthrough, switch declarations, variables used before declaration) are false positives caused by:
- The minified JSON tokenizer implementation which legitimately uses switch fallthroughs for lexer state transitions
- Webpack's module bundling patterns
These patterns are intentional and valid in the Monaco Editor source.
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/257.js (1)
1-7: Minified vendor bundle - LGTM.This is a minified Monaco Editor CSS language service bundle. It correctly creates a web worker for CSS language features and exposes the standard set of language adapters. The static analysis warnings about variables used before declaration are false positives from webpack bundling patterns.
modules/backend/formwidgets/codeeditor/assets/js/build/codeeditor.bundle/851.js (1)
1-7: TypeScript/JavaScript language service bundle - LGTM.This minified bundle provides comprehensive TypeScript and JavaScript language support for Monaco, including:
- Definition and reference navigation via
LibFileshandling- InlayHints support for parameter and type hints
- Code actions and formatting
- Diagnostics with support for
onlyVisiblemodeThe bundle correctly manages TypeScript library files (
lib.d.ts,lib.es*.d.ts, etc.) and handles the worker lifecycle with extra libs synchronization.modules/backend/formwidgets/codeeditor/assets/js/codeeditor.js (7)
61-99: LGTM!The constructor is well-structured with proper initialization of all state variables, event callbacks, and observer setup. The defensive approach of checking for
statusBarbefore accessing its child elements is good practice.
220-236: LGTM!Good use of
IntersectionObserverfor lazy loading and memory management. The editor is properly disposed when out of view and recreated when visible.
241-263: LGTM!Good approach to force a fixed height to prevent Monaco's resize issues. The jQuery backward compatibility for the Builder plugin is a thoughtful touch.
583-624: LGTM!The
wrap()andunwrap()methods correctly handle the empty selection case and use Monaco'spushEditOperationsfor atomic edits.
768-810: LGTM!Good caching strategy for themes. The silent fallback on error is acceptable since theme loading failure is non-critical and the default theme will remain active.
1291-1297: LGTM!Good memory management by disposing the editor when the tab/window is hidden. Works in conjunction with the IntersectionObserver for comprehensive visibility handling.
1308-1340: LGTM!Clever use of
constrained-editor-pluginto hide preamble content while maintaining correct line numbering. The custom line numbering function correctly adjusts displayed line numbers.
| }) | ||
|
|
||
| }) | ||
| "use strict";(self.webpackChunk_wintercms_wn_backend_module=self.webpackChunk_wintercms_wn_backend_module||[]).push([[429],{449:function(e,t,i){var n=i(171);(e=>{class t extends e.Singleton{construct(){this.widget=null}listens(){return{"backend.widget.initialized":"onWidgetInitialized"}}onWidgetInitialized(e,t){e===document.getElementById("CodeEditor-formEditorPreview-_editor_preview")&&(this.widget=t,this.enablePreferences())}enablePreferences(){(0,n.M)("change");Object.entries({show_gutter:"showGutter",highlight_active_line:"highlightActiveLine",use_hard_tabs:"!useSoftTabs",display_indent_guides:"displayIndentGuides",show_invisibles:"showInvisibles",show_print_margin:"showPrintMargin",show_minimap:"showMinimap",enable_folding:"codeFolding",bracket_colors:"bracketColors",show_colors:"showColors"}).forEach(([e,t])=>{this.element(e).addEventListener("change",e=>{this.widget.setConfig(t.replace(/^!/,""),/^!/.test(t)?!e.target.checked:e.target.checked)})}),this.element("theme").addEventListener("$change",e=>{this.widget.loadTheme(e.target.value)}),this.element("font_size").addEventListener("$change",e=>{this.widget.setConfig("fontSize",e.target.value)}),this.element("tab_size").addEventListener("$change",e=>{this.widget.setConfig("tabSize",e.target.value)}),this.element("word_wrap").addEventListener("$change",e=>{const{value:t}=e.target;switch(t){case"off":this.widget.setConfig("wordWrap",!1);break;case"fluid":this.widget.setConfig("wordWrap","fluid");break;default:this.widget.setConfig("wordWrap",parseInt(t,10))}}),document.querySelectorAll("[data-switch-lang]").forEach(e=>{e.addEventListener("click",t=>{t.preventDefault();const i=e.dataset.switchLang,n=document.querySelector(`[data-lang-snippet="${i}"]`);n&&(this.widget.setValue(n.textContent.trim()),this.widget.setLanguage(i))})}),this.widget.events.once("create",()=>{const e=new MouseEvent("click");document.querySelector('[data-switch-lang="css"]').dispatchEvent(e)})}element(e){return document.getElementById(`Form-field-Preference-editor_${e}`)}}e.addPlugin("backend.preferences",t)})(window.Snowboard)}},function(e){e.O(0,[810],function(){return t=449,e(e.s=t);var t});e.O()}]); No newline at end of file |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if there are source files that generate this bundle and verify build configuration
# Look for webpack config files
echo "=== Webpack Config Files ==="
fd -t f 'webpack\.config\.(js|ts|mjs|cjs)'
# Check for build scripts in package.json
echo -e "\n=== Build Scripts ==="
fd -t f 'package\.json' --max-depth 3 --exec jq -r '.scripts // {} | select(length > 0) | to_entries[] | select(.key | test("build|bundle|compile|watch")) | "\(.key): \(.value)"' {} \;
# Look for source files in the same directory
echo -e "\n=== Files in modules/backend/assets/js/preferences/ ==="
ls -la modules/backend/assets/js/preferences/ 2>/dev/null || echo "Directory not found"
# Look for .gitignore entries related to build output
echo -e "\n=== .gitignore entries for build output ==="
rg -n 'dist|build|bundle|/assets/' .gitignore 2>/dev/null | head -20
# Check if there's a source version of this file
echo -e "\n=== Looking for source versions ==="
fd -t f 'preferences' modules/backend/assets -E 'node_modules'
# Check the git history to see if this file was ever different
echo -e "\n=== Git info on the file ==="
git log --oneline -5 -- modules/backend/assets/js/preferences/preferences.js 2>/dev/null || echo "No git history available"Repository: wintercms/winter
Length of output: 146
🏁 Script executed:
#!/bin/bash
# Simpler check for webpack config and build setup
# Look for webpack config files (faster)
echo "=== Webpack Config Files ==="
find . -maxdepth 3 -name "webpack.config.*" -type f 2>/dev/null | head -10
# Look for package.json and extract build scripts
echo -e "\n=== Build Scripts in package.json ==="
find . -maxdepth 3 -name "package.json" -type f 2>/dev/null | head -5 | while read pf; do
echo "File: $pf"
jq '.scripts | to_entries[] | select(.key | test("build|bundle|compile")) | "\(.key): \(.value)"' "$pf" 2>/dev/null
done
# Check .gitignore
echo -e "\n=== .gitignore content (first 50 lines) ==="
head -50 .gitignore 2>/dev/null || echo ".gitignore not found"
# Check directory structure of the assets folder
echo -e "\n=== Files in modules/backend/assets/js/preferences/ ==="
find modules/backend/assets/js/preferences/ -type f 2>/dev/null | head -20
# Look for any source files nearby
echo -e "\n=== Nearby source JavaScript files ==="
find modules/backend/assets/js/ -name "*.js" -type f 2>/dev/null | head -20Repository: wintercms/winter
Length of output: 1863
🏁 Script executed:
#!/bin/bash
# Check actual build scripts and look for source files
echo "=== ./modules/backend/package.json ==="
cat ./modules/backend/package.json
echo -e "\n=== Looking for src or source directories ==="
find modules/backend -maxdepth 3 -type d \( -name "src" -o -name "source" -o -name "ts" \) 2>/dev/null
echo -e "\n=== Looking for TypeScript files ==="
find modules/backend -name "*.ts" -o -name "*.tsx" 2>/dev/null | head -10
echo -e "\n=== Check if preferences file is truly the only one in that dir ==="
stat modules/backend/assets/js/preferences/preferences.js 2>/dev/null | grep -E "File|Access|Modify|Change"
echo -e "\n=== Look for any .js source (non-minified) in assets ==="
find modules/backend/assets/js -name "*.js" ! -name "*.min.js" -type f 2>/dev/null | head -20Repository: wintercms/winter
Length of output: 2550
🏁 Script executed:
#!/bin/bash
# Look for laravel-mix config and any preferences source files
echo "=== Looking for webpack.mix.js or laravel-mix config ==="
find . -maxdepth 3 -name "webpack.mix.js" -o -name "mix.js" 2>/dev/null
echo -e "\n=== Looking for ANY source files with 'preferences' in the name ==="
find . -type f \( -name "*preferences*" -o -name "*preference*" \) ! -path "*/node_modules/*" 2>/dev/null
echo -e "\n=== Check root directory for build config ==="
ls -la webpack.* laravel-* mix.* 2>/dev/null || echo "No webpack/mix files in root"
echo -e "\n=== Check modules/backend root for build config ==="
ls -la modules/backend/webpack.* modules/backend/laravel-* modules/backend/mix.* 2>/dev/null || echo "No webpack/mix files in modules/backend"
echo -e "\n=== Looking for vue or .js source files that might compile to preferences.js ==="
find modules/backend -type f \( -name "*.vue" \) ! -path "*/node_modules/*" 2>/dev/null | head -20Repository: wintercms/winter
Length of output: 570
🏁 Script executed:
#!/bin/bash
# Check recent file modifications to understand context
echo "=== Check when preferences.js was last modified ==="
stat modules/backend/assets/js/preferences/preferences.js 2>/dev/null | grep -E "Modify|Access|File"
echo -e "\n=== Look at the file size ==="
wc -c < modules/backend/assets/js/preferences/preferences.js
echo -e "\n=== Check for any build scripts in package.json ==="
jq '.scripts' ./modules/backend/package.json
echo -e "\n=== Check root package.json for build scripts ==="
jq '.scripts' ./package.json 2>/dev/null | head -30
echo -e "\n=== Check if there's a resources or src directory at repo root ==="
find . -maxdepth 2 -type d \( -name "resources" -o -name "src" \) ! -path "*/node_modules/*" 2>/dev/nullRepository: wintercms/winter
Length of output: 329
Minified webpack bundle without source files must be removed from source control.
This is a webpack-generated bundle (self.webpackChunk_wintercms_wn_backend_module), but critically, no source file exists anywhere in the repository. The package.json includes laravel-mix as a devDependency but defines no build scripts and no webpack configuration file is present.
Committing minified build artifacts to source control causes several problems:
- Impossible to review: Minified code cannot be meaningfully reviewed for correctness or security.
- Unmaintainable: Changes must be made in missing source files; this committed artifact is redundant.
- Merge conflicts: Minified files frequently create difficult-to-resolve conflicts.
- No build process: Without build configuration, this file cannot be regenerated.
Required actions:
- Locate or reconstruct the original source file for this module.
- Set up the webpack/laravel-mix build configuration.
- Remove
modules/backend/assets/js/preferences/preferences.jsfrom source control and add build output to.gitignore. - Ensure the build process generates this file during CI/CD or deployment.
🧰 Tools
🪛 Biome (2.1.2)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
| 'yaml_emit_file|yaml_parse|yaml_parse_file|yaml_parse_url|yaz_addinfo|yaz_ccl_conf|yaz_ccl_parse|yaz_close|yaz_connect|yaz_database|'+'yaz_element|yaz_errno|yaz_error|yaz_es|yaz_es_result|yaz_get_option|yaz_hits|yaz_itemorder|yaz_present|yaz_range|yaz_record|yaz_scan|'+'yaz_scan_result|yaz_schema|yaz_search|yaz_set_option|yaz_sort|yaz_syntax|yaz_wait|yp_all|yp_cat|yp_err_string|yp_errno|yp_first|'+'yp_get_default_domain|yp_master|yp_match|yp_next|yp_order|zend_logo_guid|zend_thread_id|zend_version|zip_close|zip_entry_close|'+'zip_entry_compressedsize|zip_entry_compressionmethod|zip_entry_filesize|zip_entry_name|zip_entry_open|zip_entry_read|zip_open|zip_read|'+'ziparchive|ziparchive_addemptydir|ziparchive_addfile|ziparchive_addfromstring|ziparchive_close|ziparchive_deleteindex|'+'ziparchive_deletename|ziparchive_extractto|ziparchive_getarchivecomment|ziparchive_getcommentindex|ziparchive_getcommentname|'+ | ||
| 'ziparchive_getfromindex|ziparchive_getfromname|ziparchive_getnameindex|ziparchive_getstatusstring|ziparchive_getstream|'+'ziparchive_locatename|ziparchive_open|ziparchive_renameindex|ziparchive_renamename|ziparchive_setCommentName|ziparchive_setarchivecomment|'+'ziparchive_setcommentindex|ziparchive_statindex|ziparchive_statname|ziparchive_unchangeall|ziparchive_unchangearchive|'+'ziparchive_unchangeindex|ziparchive_unchangename|zlib_get_coding_type').split('|'));var keywords=lang.arrayToMap(('abstract|and|array|as|break|case|catch|class|clone|const|continue|declare|default|do|else|elseif|enddeclare|endfor|endforeach|endif|'+'endswitch|endwhile|extends|final|for|foreach|function|global|goto|if|implements|interface|instanceof|namespace|new|or|private|protected|'+'public|static|switch|throw|trait|try|use|var|while|xor').split('|'));var languageConstructs=lang.arrayToMap(('die|echo|empty|exit|eval|include|include_once|isset|list|require|require_once|return|print|unset').split('|'));var builtinConstants=lang.arrayToMap( | ||
| ('true|TRUE|false|FALSE|null|NULL|__CLASS__|__DIR__|../vendor/ace/mode-php.js|__LINE__|__METHOD__|__FUNCTION__|__NAMESPACE__').split('|'));var builtinVariables=lang.arrayToMap(('$GLOBALS|$_SERVER|$_GET|$_POST|$_FILES|$_REQUEST|$_SESSION|$_ENV|$_COOKIE|$php_errormsg|$HTTP_RAW_POST_DATA|'+'$http_response_header|$argc|$argv').split('|'));var builtinFunctionsDeprecated=lang.arrayToMap(('key_exists|cairo_matrix_create_scale|cairo_matrix_create_translate|call_user_method|call_user_method_array|com_addref|com_get|'+'com_invoke|com_isenum|com_load|com_release|com_set|connection_timeout|cubrid_load_from_glo|cubrid_new_glo|cubrid_save_to_glo|'+'cubrid_send_glo|define_syslog_variables|dl|ereg|ereg_replace|eregi|eregi_replace|hw_documentattributes|hw_documentbodytag|'+'hw_documentsize|hw_outputdocument|imagedashedline|maxdb_bind_param|maxdb_bind_result|maxdb_client_encoding|maxdb_close_long_data|'+ | ||
| ('true|TRUE|false|FALSE|null|NULL|__CLASS__|__DIR__|../ace/mode-php.js|__LINE__|__METHOD__|__FUNCTION__|__NAMESPACE__').split('|'));var builtinVariables=lang.arrayToMap(('$GLOBALS|$_SERVER|$_GET|$_POST|$_FILES|$_REQUEST|$_SESSION|$_ENV|$_COOKIE|$php_errormsg|$HTTP_RAW_POST_DATA|'+'$http_response_header|$argc|$argv').split('|'));var builtinFunctionsDeprecated=lang.arrayToMap(('key_exists|cairo_matrix_create_scale|cairo_matrix_create_translate|call_user_method|call_user_method_array|com_addref|com_get|'+'com_invoke|com_isenum|com_load|com_release|com_set|connection_timeout|cubrid_load_from_glo|cubrid_new_glo|cubrid_save_to_glo|'+'cubrid_send_glo|define_syslog_variables|dl|ereg|ereg_replace|eregi|eregi_replace|hw_documentattributes|hw_documentbodytag|'+'hw_documentsize|hw_outputdocument|imagedashedline|maxdb_bind_param|maxdb_bind_result|maxdb_client_encoding|maxdb_close_long_data|'+ |
There was a problem hiding this comment.
Critical: File path incorrectly placed in PHP builtinConstants array.
The string ../ace/mode-php.js appears in the builtinConstants array, which should only contain PHP magic constants like __CLASS__, __DIR__, __FILE__, etc. This looks like the __FILE__ constant was accidentally replaced during a find-replace operation when updating asset paths.
This will break syntax highlighting and code intelligence for the __FILE__ magic constant in PHP code.
🔎 Proposed fix
-('true|TRUE|false|FALSE|null|NULL|__CLASS__|__DIR__|../ace/mode-php.js|__LINE__|__METHOD__|__FUNCTION__|__NAMESPACE__').split('|'));
+('true|TRUE|false|FALSE|null|NULL|__CLASS__|__DIR__|__FILE__|__LINE__|__METHOD__|__FUNCTION__|__NAMESPACE__').split('|'));📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ('true|TRUE|false|FALSE|null|NULL|__CLASS__|__DIR__|../ace/mode-php.js|__LINE__|__METHOD__|__FUNCTION__|__NAMESPACE__').split('|'));var builtinVariables=lang.arrayToMap(('$GLOBALS|$_SERVER|$_GET|$_POST|$_FILES|$_REQUEST|$_SESSION|$_ENV|$_COOKIE|$php_errormsg|$HTTP_RAW_POST_DATA|'+'$http_response_header|$argc|$argv').split('|'));var builtinFunctionsDeprecated=lang.arrayToMap(('key_exists|cairo_matrix_create_scale|cairo_matrix_create_translate|call_user_method|call_user_method_array|com_addref|com_get|'+'com_invoke|com_isenum|com_load|com_release|com_set|connection_timeout|cubrid_load_from_glo|cubrid_new_glo|cubrid_save_to_glo|'+'cubrid_send_glo|define_syslog_variables|dl|ereg|ereg_replace|eregi|eregi_replace|hw_documentattributes|hw_documentbodytag|'+'hw_documentsize|hw_outputdocument|imagedashedline|maxdb_bind_param|maxdb_bind_result|maxdb_client_encoding|maxdb_close_long_data|'+ | |
| ('true|TRUE|false|FALSE|null|NULL|__CLASS__|__DIR__|__FILE__|__LINE__|__METHOD__|__FUNCTION__|__NAMESPACE__').split('|'));var builtinVariables=lang.arrayToMap(('$GLOBALS|$_SERVER|$_GET|$_POST|$_FILES|$_REQUEST|$_SESSION|$_ENV|$_COOKIE|$php_errormsg|$HTTP_RAW_POST_DATA|'+'$http_response_header|$argc|$argv').split('|'));var builtinFunctionsDeprecated=lang.arrayToMap(('key_exists|cairo_matrix_create_scale|cairo_matrix_create_translate|call_user_method|call_user_method_array|com_addref|com_get|'+'com_invoke|com_isenum|com_load|com_release|com_set|connection_timeout|cubrid_load_from_glo|cubrid_new_glo|cubrid_save_to_glo|'+'cubrid_send_glo|define_syslog_variables|dl|ereg|ereg_replace|eregi|eregi_replace|hw_documentattributes|hw_documentbodytag|'+'hw_documentsize|hw_outputdocument|imagedashedline|maxdb_bind_param|maxdb_bind_result|maxdb_client_encoding|maxdb_close_long_data|'+ |
🤖 Prompt for AI Agents
In modules/backend/assets/vendor/ace-codeeditor/build-min.js around line 1442,
the builtinConstants array contains an incorrect token "../ace/mode-php.js"
instead of the PHP magic constant "__FILE__"; remove the erroneous path entry
and restore the correct "__FILE__" token in the array so only PHP magic
constants remain (ensure formatting/quoting matches surrounding entries and
update any adjacent concatenation so the split('|') still produces the correct
list).
| $(document).render(function(){$('[data-control="ace-codeeditor"]').codeEditor()});+function(exports){if(exports.ace&&typeof exports.ace.require=='function'){var emmetExt=exports.ace.require('ace/ext/emmet') | ||
| if(emmetExt&&emmetExt.AceEmmetEditor&&emmetExt.AceEmmetEditor.prototype.getSyntax){var coreGetSyntax=emmetExt.AceEmmetEditor.prototype.getSyntax | ||
| emmetExt.AceEmmetEditor.prototype.getSyntax=function(){var $syntax=$.proxy(coreGetSyntax,this)() | ||
| return $syntax=='twig'?'html':$syntax};}}}(window)}(window.jQuery); No newline at end of file |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Search for data-control attributes related to code editors across the codebase
rg -n 'data-control\s*=\s*["\'].*codeeditor' --type-list | grep -E '\.(php|js|html|vue)' -q && echo "Has codeeditor references" || true
rg -n 'data-control\s*=\s*["\'].*codeeditor' -C 2
# Also search for Monaco references to understand migration context
rg -n 'monaco|Monaco' --type-list | head -20Repository: wintercms/winter
Length of output: 282
🏁 Script executed:
# Search for references to the old 'codeeditor' data attribute without ace prefix
rg -n 'data-control=["\']codeeditor["\']' -C 2
# Search for 'ace-codeeditor' references to confirm new selector usage
rg -n 'ace-codeeditor' -C 1Repository: wintercms/winter
Length of output: 128
🏁 Script executed:
# Check for JavaScript initialization code that may reference these selectors
rg -n '\$.*codeeditor|\$.*ace-codeeditor|codeEditor\(\)' --type=js -C 3 | head -100Repository: wintercms/winter
Length of output: 50372
🏁 Script executed:
# Search for old 'codeeditor' selector in data-control attributes specifically (case-insensitive)
rg 'data-control\s*=\s*["\']codeeditor["\']' -i
# Search for all data-control selectors to see what's registered
rg 'data-control\s*=\s*["\']' | grep -E '(ace-|monaco|codeeditor)' | head -20Repository: wintercms/winter
Length of output: 224
🏁 Script executed:
# Check if there are any form widget definitions that might reference old selectors
fd -e php -e js | xargs rg -l 'codeeditor|code-editor' | head -20Repository: wintercms/winter
Length of output: 989
🏁 Script executed:
# Simple search for old selector pattern (without ace- prefix)
rg 'data-control.*codeeditor' --no-config | grep -v 'ace-codeeditor'
# Also check the form widget class definition
cat modules/backend/formwidgets/CodeEditor.php | head -50Repository: wintercms/winter
Length of output: 1965
🏁 Script executed:
# Check the partial template file to see what selector is used
cat modules/backend/formwidgets/codeeditor/partials/_codeeditor.phpRepository: wintercms/winter
Length of output: 2288
🏁 Script executed:
# Verify the Emmet integration code in the source file
sed -n '444,455p' modules/backend/assets/vendor/ace-codeeditor/codeeditor.jsRepository: wintercms/winter
Length of output: 459
Selector mismatch breaks the Ace code editor initialization.
The vendored Ace code attempts to initialize elements with data-control="ace-codeeditor" (lines 2311-2314), but the form widget template at modules/backend/formwidgets/codeeditor/partials/_codeeditor.php still renders data-control="codeeditor". This mismatch prevents the initialization from working. Additionally, other parts of the codebase still reference the old [data-control="codeeditor"] selector (e.g., modules/cms/assets/js/winter.cmspage.js, modules/system/assets/ui/js/drag.value.js), creating potential breakage across multiple features.
The form widget template must be updated to use data-control="ace-codeeditor", and any dependent selectors in related modules must be updated accordingly.
| /** | ||
| * Loads a theme via AJAX. | ||
| * Supports both tmTheme (XML) and JSON formats. | ||
| */ | ||
| public function onLoadTheme() | ||
| { | ||
| $theme = post('theme'); | ||
|
|
||
| if (empty($theme)) { | ||
| throw new ApplicationException('No theme specified'); | ||
| } | ||
| if (!preg_match('/^[a-z\-\_]+$/i', $theme)) { | ||
| throw new ApplicationException('Invalid theme name'); | ||
| } | ||
|
|
||
| $themeDir = __DIR__ . '/codeeditor/assets/themes/'; | ||
|
|
||
| // Try JSON format first (modern), then fall back to tmTheme (legacy) | ||
| $jsonPath = $themeDir . $theme . '.json'; | ||
| $tmThemePath = $themeDir . $theme . '.tmTheme'; | ||
|
|
||
| if (File::exists($jsonPath)) { | ||
| return [ | ||
| 'format' => 'json', | ||
| 'data' => File::get($jsonPath), | ||
| ]; | ||
| } elseif (File::exists($tmThemePath)) { | ||
| return [ | ||
| 'format' => 'tmTheme', | ||
| 'data' => File::get($tmThemePath), | ||
| ]; | ||
| } | ||
|
|
||
| throw new ApplicationException(sprintf('Theme "%s" not found', $theme)); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find modules/backend/formwidgets/codeeditor/assets/themes/ -type f \( -name "*.json" -o -name "*.tmTheme" \) 2>/dev/null | sortRepository: wintercms/winter
Length of output: 2650
Theme loading needs case-normalization to work reliably on case-sensitive filesystems.
The onLoadTheme() method validates input appropriately and constructs paths safely, but has a case-sensitivity issue: the regex /^[a-z\-\_]+$/i (with the i flag) accepts mixed-case input like "Monokai" or "THEMES", but all theme files in the directory are lowercase (e.g., monokai.tmTheme, one-dark-pro.json, kr_theme.tmTheme).
On case-sensitive filesystems (Linux), theme loading fails if users provide mixed-case input. On case-insensitive filesystems (Windows/macOS), it works by accident.
Fix: Convert the theme name to lowercase before path construction:
$theme = strtolower(post('theme'));This ensures consistent behavior across all filesystems.
🤖 Prompt for AI Agents
In modules/backend/formwidgets/CodeEditor.php around lines 201 to 235, the
onLoadTheme() handler accepts mixed-case theme names (regex uses /i) but theme
files are lowercase, causing failures on case-sensitive filesystems; normalize
the input by lowercasing the posted theme before validation/path construction
(e.g., call strtolower on post('theme')) and then proceed with the existing
validation and File::exists checks so paths match actual filenames.
| findAll(search, matchCase) { | ||
| const searchString = (search instanceof RegExp) ? search.source : search; | ||
| const matches = this.model.findMatches(searchString, true, (search instanceof RegExp), matchCase || false, null, true, 1); | ||
|
|
||
| // CODEEDITOR NO CONFLICT | ||
| // ================= | ||
| return matches; | ||
| } |
There was a problem hiding this comment.
findAll() only returns one match due to limit parameter.
The findMatches() call has limitResultCount set to 1, which contradicts the method's purpose of finding all matches.
🔎 Proposed fix
findAll(search, matchCase) {
const searchString = (search instanceof RegExp) ? search.source : search;
- const matches = this.model.findMatches(searchString, true, (search instanceof RegExp), matchCase || false, null, true, 1);
+ const matches = this.model.findMatches(searchString, true, (search instanceof RegExp), matchCase || false, null, true);
return matches;
}Note: Omitting the last parameter removes the limit, returning all matches.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| findAll(search, matchCase) { | |
| const searchString = (search instanceof RegExp) ? search.source : search; | |
| const matches = this.model.findMatches(searchString, true, (search instanceof RegExp), matchCase || false, null, true, 1); | |
| // CODEEDITOR NO CONFLICT | |
| // ================= | |
| return matches; | |
| } | |
| findAll(search, matchCase) { | |
| const searchString = (search instanceof RegExp) ? search.source : search; | |
| const matches = this.model.findMatches(searchString, true, (search instanceof RegExp), matchCase || false, null, true); | |
| return matches; | |
| } |
🤖 Prompt for AI Agents
In modules/backend/formwidgets/codeeditor/assets/js/codeeditor.js around lines
644 to 649, the call to this.model.findMatches sets the last parameter
limitResultCount to 1, causing findAll() to return only one match; remove the
final numeric argument (i.e., omit the limitResultCount parameter) so
findMatches is called without a limit and returns all matches.
| return this.model.pushEditOperations( | ||
| this.editor.getSelections(), | ||
| [ | ||
| { | ||
| forceMoveMarkers: false, | ||
| range: new monaco.Range(found.range.startLineNumber, found.range.startColumn, found.range.endLineNumber, found.range.endColumn), | ||
| text: (align) ? this.alignText(replace, search.startColumn) : replace, | ||
| }, | ||
| ], | ||
| ); | ||
| } |
There was a problem hiding this comment.
Incorrect property access for alignment when search is String/RegExp.
On line 685, search.startColumn is used for alignment, but when search is a String or RegExp (not a Range), this property is undefined, causing the alignment to fail silently.
🔎 Proposed fix
return this.model.pushEditOperations(
this.editor.getSelections(),
[
{
forceMoveMarkers: false,
range: new monaco.Range(found.range.startLineNumber, found.range.startColumn, found.range.endLineNumber, found.range.endColumn),
- text: (align) ? this.alignText(replace, search.startColumn) : replace,
+ text: (align) ? this.alignText(replace, found.range.startColumn) : replace,
},
],
);🤖 Prompt for AI Agents
In modules/backend/formwidgets/codeeditor/assets/js/codeeditor.js around lines
679 to 689, the code uses search.startColumn for alignment which is undefined
when search is a String or RegExp; replace that usage with the actual matched
range column (e.g., found.range.startColumn) so alignment always gets a valid
column value; update the align call to pass found.range.startColumn (or
otherwise guard by checking for a Range and fallback to found.range.startColumn)
instead of search.startColumn.
| enableStatusBarActions() { | ||
| if (!this.statusBar) { | ||
| return; | ||
| } | ||
|
|
||
| const fullscreen = this.statusBar.querySelector('[data-full-screen]'); | ||
| fullscreen.addEventListener('click', () => { | ||
| if (!this.fullscreen) { | ||
| this.element.requestFullscreen({ | ||
| navigationUI: 'hide', | ||
| }).then(() => { | ||
| this.fullscreen = true; | ||
| fullscreen.classList.add('active'); | ||
| this.element.addEventListener('fullscreenchange', this.callbacks.fullScreenChange); | ||
| // Use Monaco's layout() instead of refresh() to avoid disposing the editor | ||
| if (this.editor) { | ||
| window.requestAnimationFrame(() => { | ||
| this.editor.layout(); | ||
| }); | ||
| } | ||
| }); | ||
| } else { | ||
| document.exitFullscreen(); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Click listener on fullscreen button accumulates on editor recreation.
enableStatusBarActions() is called every time createEditor() runs (on visibility changes), but there's no cleanup of the previous click listener on the fullscreen button. This causes multiple listeners to accumulate.
🔎 Proposed fix
Track and clean up the fullscreen listener:
+ // In construct(), add:
+ this.fullscreenClickHandler = null;
enableStatusBarActions() {
if (!this.statusBar) {
return;
}
const fullscreen = this.statusBar.querySelector('[data-full-screen]');
+
+ // Remove previous listener if exists
+ if (this.fullscreenClickHandler) {
+ fullscreen.removeEventListener('click', this.fullscreenClickHandler);
+ }
+
+ this.fullscreenClickHandler = () => {
- fullscreen.addEventListener('click', () => {
if (!this.fullscreen) {
// ... existing code
} else {
document.exitFullscreen();
}
- });
+ };
+
+ fullscreen.addEventListener('click', this.fullscreenClickHandler);
}| normalizeKeyBinding(keyCode) { | ||
| let keyBinding = { | ||
| key: null, | ||
| ctrl: false, | ||
| alt: false, | ||
| shift: false, | ||
| }; | ||
|
|
||
| if (typeof keyCode === 'string') { | ||
| if (keyCode.startsWith('Shift+Ctrl+')) { | ||
| keyBinding.key = keyCode.replace('Shift+Ctrl+', ''); | ||
| keyBinding.shift = true; | ||
| keyBinding.ctrl = true; | ||
| } else if (keyCode.startsWith('Shift+Alt+')) { | ||
| keyBinding.key = keyCode.replace('Shift+Alt+', ''); | ||
| keyBinding.shift = true; | ||
| keyBinding.alt = true; | ||
| } else if (keyCode.startsWith('Ctrl+')) { | ||
| keyBinding.key = keyCode.replace('Ctrl+', ''); | ||
| keyBinding.ctrl = true; | ||
| } else if (keyCode.startsWith('Alt+')) { | ||
| keyBinding.key = keyCode.replace('Alt+', ''); | ||
| keyBinding.alt = true; | ||
| } | ||
| } else { | ||
| keyBinding = { ...keyBinding, ...keyCode }; | ||
| } | ||
|
|
||
| return keyBinding; | ||
| } |
There was a problem hiding this comment.
Incomplete keybinding parsing for some combinations.
The parser doesn't handle:
- Plain keys without modifiers (e.g.,
"Escape") —keyremainsnull Shift+alone without Ctrl/AltCtrl+Alt+combinations- Alternative orderings like
Ctrl+Shift+
🔎 Proposed fix
normalizeKeyBinding(keyCode) {
let keyBinding = {
key: null,
ctrl: false,
alt: false,
shift: false,
};
if (typeof keyCode === 'string') {
- if (keyCode.startsWith('Shift+Ctrl+')) {
- keyBinding.key = keyCode.replace('Shift+Ctrl+', '');
- keyBinding.shift = true;
- keyBinding.ctrl = true;
- } else if (keyCode.startsWith('Shift+Alt+')) {
- keyBinding.key = keyCode.replace('Shift+Alt+', '');
- keyBinding.shift = true;
- keyBinding.alt = true;
- } else if (keyCode.startsWith('Ctrl+')) {
- keyBinding.key = keyCode.replace('Ctrl+', '');
- keyBinding.ctrl = true;
- } else if (keyCode.startsWith('Alt+')) {
- keyBinding.key = keyCode.replace('Alt+', '');
- keyBinding.alt = true;
- }
+ const parts = keyCode.split('+');
+ keyBinding.key = parts.pop(); // Last part is the key
+ parts.forEach((mod) => {
+ const modifier = mod.toLowerCase();
+ if (modifier === 'ctrl' || modifier === 'cmd') keyBinding.ctrl = true;
+ else if (modifier === 'alt') keyBinding.alt = true;
+ else if (modifier === 'shift') keyBinding.shift = true;
+ });
} else {
keyBinding = { ...keyBinding, ...keyCode };
}
return keyBinding;
}🤖 Prompt for AI Agents
In modules/backend/formwidgets/codeeditor/assets/js/codeeditor.js around lines
1419 to 1448, the normalizeKeyBinding function fails to parse many valid string
keybindings (plain keys like "Escape", single "Shift+Key", "Ctrl+Alt+Key", and
modifier permutations like "Ctrl+Shift+Key"), so replace the current startsWith
chain with a robust parser: split the input string on '+' into parts, iterate
parts to set ctrl/alt/shift flags when matching modifier names
(case-insensitive), and treat the remaining non-modifier part (if any) as the
key; ensure empty or missing key parts result in key = null and preserve
existing behavior for non-string inputs by merging the parsed result into the
default keyBinding object.
| } | ||
|
|
||
| $this->addJs('/modules/backend/formwidgets/codeeditor/assets/js/build-min.js', 'core'); | ||
| $this->addJs('/modules/backend/assets/vendor/ace-codeeditor/build-min.js', 'core'); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify the ace-codeeditor asset exists at the new path
# Check if the ace-codeeditor build file exists
if [ -f "modules/backend/assets/vendor/ace-codeeditor/build-min.js" ]; then
echo "✓ Asset file exists at modules/backend/assets/vendor/ace-codeeditor/build-min.js"
ls -lh "modules/backend/assets/vendor/ace-codeeditor/build-min.js"
else
echo "✗ Asset file NOT found at modules/backend/assets/vendor/ace-codeeditor/build-min.js"
echo "Searching for ace-codeeditor files in the repository..."
fd -t f "build-min.js" modules/backend/assets/vendor/
fi
# Check if there are any open issues or TODOs tracking the migration of RichEditor to Monaco
echo ""
echo "Checking for TODOs or FIXMEs related to RichEditor and Monaco migration..."
rg -i "TODO|FIXME" modules/backend/formwidgets/RichEditor.phpRepository: wintercms/winter
Length of output: 323
🏁 Script executed:
#!/bin/bash
# Search for tracking of temporary Ace retention in the codebase
echo "=== Searching for Ace/Monaco migration tracking in backend formwidgets ==="
rg -i "temporary|temp|ace.*monaco|monaco.*ace|fixme|todo" modules/backend/formwidgets/ -A 2 -B 2
echo ""
echo "=== Searching in RichEditor.php for any comments about Ace vs Monaco ==="
cat -n modules/backend/formwidgets/RichEditor.php | head -200
echo ""
echo "=== Checking commit messages or PR history for migration context ==="
rg "temporary.*ace|ace.*retained" --max-count=10 -iRepository: wintercms/winter
Length of output: 50375
🏁 Script executed:
#!/bin/bash
# Read RichEditor.php to check for comments about Ace editor
cat -n modules/backend/formwidgets/RichEditor.php | head -180
# Also search for any comments mentioning Ace, Monaco, or temporary
echo ""
echo "=== Searching for comments about Ace/Monaco in RichEditor.php ==="
rg -i "ace|monaco|editor|temporary" modules/backend/formwidgets/RichEditor.php -B 2 -A 2Repository: wintercms/winter
Length of output: 11446
🏁 Script executed:
#!/bin/bash
# Search for any migration tracking across the entire codebase
echo "=== Searching for Monaco/Ace migration references ==="
rg -i "monaco|ace.*temporary|temporary.*ace" --max-count=20
echo ""
echo "=== Checking MarkdownEditor for similar Ace loading ==="
rg -A 5 -B 5 "ace-codeeditor" modules/backend/formwidgets/MarkdownEditor.php
echo ""
echo "=== Looking for any issue/TODO tracking in commit messages or PR files ==="
rg -i "TODO|FIXME|issue" modules/backend/formwidgets/ -l | head -10Repository: wintercms/winter
Length of output: 50374
The Ace editor asset path is correct, but document that this Ace retention is temporary.
The asset file at /modules/backend/assets/vendor/ace-codeeditor/build-min.js exists and loads correctly. However, RichEditor still retains Ace instead of migrating to Monaco, with no inline documentation or tracking indicating this is a temporary measure. Add a comment in the code or create/link a follow-up issue to ensure this temporary retention doesn't become permanent technical debt.
🤖 Prompt for AI Agents
In modules/backend/formwidgets/RichEditor.php around line 157, the code adds the
Ace editor asset but lacks documentation that keeping Ace is a temporary
measure; add a clear inline comment on that line stating that Ace is retained
temporarily pending migration to Monaco, include the rationale and expected
timeline or milestone, and either add a TODO with a tracker/issue ID (or create
one and reference it) so the retention is traceable and won't become permanent
technical debt.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
modules/backend/formwidgets/codeeditor/README.md (2)
134-162: Specify language identifier for the architecture diagram fenced code block.The ASCII-art file structure diagram lacks a language identifier on the opening fence. While it renders correctly without one, markdownlint and best practices recommend specifying a language even for non-code text blocks.
Proposed fix
-``` +```text modules/backend/formwidgets/codeeditor/ ├── assets/
590-597: Wrap bare URLs in markdown link syntax.All 6 resource URLs (lines 592–597) are bare and should be formatted as markdown links for better rendering, accessibility, and to satisfy the markdown linter (MD034).
Proposed fix
- **Monaco Editor Documentation:** https://microsoft.github.io/monaco-editor/ - **Monaco Editor GitHub:** https://github.com/microsoft/monaco-editor - **VS Code Themes:** https://marketplace.visualstudio.com/search?target=VSCode&category=Themes - **Winter CMS Docs:** https://wintercms.com/docs - **PR #801:** https://github.com/wintercms/winter/pull/801 - **Issue #431:** https://github.com/wintercms/winter/issues/431 + **Monaco Editor Documentation:** [https://microsoft.github.io/monaco-editor/](https://microsoft.github.io/monaco-editor/) + **Monaco Editor GitHub:** [https://github.com/microsoft/monaco-editor](https://github.com/microsoft/monaco-editor) + **VS Code Themes:** [https://marketplace.visualstudio.com/search?target=VSCode&category=Themes](https://marketplace.visualstudio.com/search?target=VSCode&category=Themes) + **Winter CMS Docs:** [https://wintercms.com/docs](https://wintercms.com/docs) + **PR #801:** [https://github.com/wintercms/winter/pull/801](https://github.com/wintercms/winter/pull/801) + **Issue #431:** [https://github.com/wintercms/winter/issues/431](https://github.com/wintercms/winter/issues/431)
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (3)
modules/backend/formwidgets/colorpicker/assets/js/dist/colorpicker.jsis excluded by!**/dist/**modules/backend/formwidgets/iconpicker/assets/js/dist/iconpicker.jsis excluded by!**/dist/**modules/backend/formwidgets/sensitive/assets/js/dist/sensitive.jsis excluded by!**/dist/**
📒 Files selected for processing (4)
modules/backend/assets/ui/js/build/backend.jsmodules/backend/assets/ui/js/build/manifest.jsmodules/backend/formwidgets/codeeditor/README.mdmodules/backend/formwidgets/codeeditor/assets/winter.mix.js
🧰 Additional context used
🧬 Code graph analysis (1)
modules/backend/assets/ui/js/build/backend.js (1)
modules/system/assets/js/snowboard/main/Snowboard.js (1)
Snowboard(21-595)
🪛 Biome (2.1.2)
modules/backend/assets/ui/js/build/backend.js
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
modules/backend/assets/ui/js/build/manifest.js
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
🪛 markdownlint-cli2 (0.18.1)
modules/backend/formwidgets/codeeditor/README.md
134-134: Fenced code blocks should have a language specified
(MD040, fenced-code-language)
592-592: Bare URL used
(MD034, no-bare-urls)
593-593: Bare URL used
(MD034, no-bare-urls)
594-594: Bare URL used
(MD034, no-bare-urls)
595-595: Bare URL used
(MD034, no-bare-urls)
596-596: Bare URL used
(MD034, no-bare-urls)
597-597: Bare URL used
(MD034, no-bare-urls)
⏰ 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). (9)
- GitHub Check: windows-latest / PHP 8.1
- GitHub Check: ubuntu-latest / PHP 8.1
- GitHub Check: windows-latest / PHP 8.2
- GitHub Check: ubuntu-latest / PHP 8.3
- GitHub Check: windows-latest / PHP 8.3
- GitHub Check: windows-latest / PHP 8.4
- GitHub Check: ubuntu-latest / PHP 8.2
- GitHub Check: ubuntu-latest / PHP 8.4
- GitHub Check: windows-latest / JavaScript
🔇 Additional comments (1)
modules/backend/formwidgets/codeeditor/README.md (1)
1-597: Comprehensive and well-structured documentation for Monaco Editor migration.The README provides excellent coverage of the migration from ACE to Monaco, including clear explanation of the dual-editor architecture, comprehensive API reference, detailed migration guide with breaking changes, real-world examples from Winter.Builder, and testing guidance. The migration checklist and API comparison table are particularly helpful for developers updating their code. Overall documentation quality is high and should effectively guide both users and plugin developers through the transition.
| @@ -1 +1 @@ | |||
| "use strict";(self.webpackChunk_wintercms_wn_backend_module=self.webpackChunk_wintercms_wn_backend_module||[]).push([[476],{286:function(e,t,n){var i=n(35),r=n(171);class s extends Snowboard.Singleton{listens(){return{ready:"ready",ajaxFetchOptions:"ajaxFetchOptions",ajaxUpdateComplete:"ajaxUpdateComplete"}}ready(){window.jQuery&&((0,r.M)("render"),document.addEventListener("$render",(()=>{this.snowboard.globalEvent("render")})),window.jQuery(document).trigger("render"))}addPrefilter(){window.jQuery&&window.jQuery.ajaxPrefilter((e=>{this.hasToken()&&(e.headers||(e.headers={}),e.headers["X-CSRF-TOKEN"]=this.getToken())}))}ajaxFetchOptions(e){this.hasToken()&&(e.headers["X-CSRF-TOKEN"]=this.getToken())}ajaxUpdateComplete(){window.jQuery&&window.jQuery(document).trigger("render")}hasToken(){const e=document.querySelector('meta[name="csrf-token"]');return!!e&&!!e.hasAttribute("content")}getToken(){return document.querySelector('meta[name="csrf-token"]').getAttribute("content")}}class a extends Snowboard.PluginBase{construct(e,t){if(e instanceof Snowboard.PluginBase==!1)throw new Error("Event handling can only be applied to Snowboard classes.");if(!t)throw new Error("Event prefix is required.");this.instance=e,this.eventPrefix=t,this.events=[]}on(e,t){this.events.push({event:e,callback:t})}off(e,t){this.events=this.events.filter((n=>n.event!==e||n.callback!==t))}once(e,t){var n=this;const i=this.events.push({event:e,callback:function(){t(...arguments),n.events.splice(i-1,1)}})}fire(e){for(var t=arguments.length,n=new Array(t>1?t-1:0),i=1;i<t;i++)n[i-1]=arguments[i];const r=this.events.filter((t=>t.event===e));let s=!1;r.forEach((e=>{s||!1===e.callback(...n)&&(s=!0)})),s||this.snowboard.globalEvent(`${this.eventPrefix}.${e}`,...n)}firePromise(e){for(var t=arguments.length,n=new Array(t>1?t-1:0),i=1;i<t;i++)n[i-1]=arguments[i];const r=this.events.filter((t=>t.event===e)),s=r.filter((e=>null!==e),r.map((e=>e.callback(...n))));Promise.all(s).then((()=>{this.snowboard.globalPromiseEvent(`${this.eventPrefix}.${e}`,...n)}))}}class o extends Snowboard.Singleton{construct(){this.registeredWidgets=[],this.elements=[],this.events={mutate:e=>this.onMutation(e)},this.observer=null}listens(){return{ready:"onReady",render:"onRender",ajaxUpdate:"onAjaxUpdate"}}register(e,t,n){this.registeredWidgets.push({control:e,widget:t,callback:n})}unregister(e){this.registeredWidgets=this.registeredWidgets.filter((t=>t.control!==e))}onReady(){this.initializeWidgets(document.body),this.observer||(this.observer=new MutationObserver(this.events.mutate),this.observer.observe(document.body,{childList:!0,subtree:!0}))}onRender(){this.initializeWidgets(document.body)}onAjaxUpdate(e){this.initializeWidgets(e)}initializeWidgets(e){this.registeredWidgets.forEach((t=>{const n=e.querySelectorAll(`[data-control="${t.control}"]:not([data-widget-initialized])`);n.length&&n.forEach((e=>{if(e.dataset.widgetInitialized)return;const n=this.snowboard[t.widget](e);this.elements.push({element:e,instance:n}),e.dataset.widgetInitialized=!0,this.snowboard.globalEvent("backend.widget.initialized",e,n),"function"==typeof t.callback&&t.callback(n,e)}))}))}getWidget(e){const t=this.elements.find((t=>t.element===e));return t?t.instance:null}onMutation(e){const t=e.filter((e=>e.removedNodes.length)).map((e=>Array.from(e.removedNodes))).flat();t.length&&t.forEach((e=>{const t=this.elements.filter((t=>e.contains(t.element)));t.length&&t.forEach((e=>{e.instance.destruct(),this.elements=this.elements.filter((t=>t!==e))}))}))}}if(void 0===window.Snowboard)throw new Error("Snowboard must be loaded in order to use the Backend UI.");(e=>{e.addPlugin("backend.ajax.handler",s),e.addPlugin("backend.ui.eventHandler",a),e.addPlugin("backend.ui.widgetHandler",o),e["backend.ajax.handler"]().addPrefilter(),window.AssetManager={load:(t,n)=>{e.assetLoader().load(t).then((()=>{n&&"function"==typeof n&&n()}))}},window.assetManager=window.AssetManager})(window.Snowboard),window.Vue=i}},function(e){e.O(0,[429],(function(){return t=286,e(e.s=t);var t}));e.O()}]); No newline at end of file | |||
| "use strict";(self.webpackChunk_wintercms_wn_backend_module=self.webpackChunk_wintercms_wn_backend_module||[]).push([[476],{286:function(e,t,n){var i=n(35),r=n(171);class s extends Snowboard.Singleton{listens(){return{ready:"ready",ajaxFetchOptions:"ajaxFetchOptions",ajaxUpdateComplete:"ajaxUpdateComplete"}}ready(){window.jQuery&&((0,r.M)("render"),(0,r.M)("render"),document.addEventListener("$render",()=>{this.snowboard.globalEvent("render")}),window.jQuery(document).trigger("render"),document.addEventListener("$render",()=>{this.snowboard.globalEvent("render")}))}addPrefilter(){window.jQuery&&window.jQuery.ajaxPrefilter(e=>{this.hasToken()&&(e.headers||(e.headers={}),e.headers["X-CSRF-TOKEN"]=this.getToken())})}ajaxFetchOptions(e){this.hasToken()&&(e.headers["X-CSRF-TOKEN"]=this.getToken())}ajaxUpdateComplete(){window.jQuery&&window.jQuery(document).trigger("render")}hasToken(){const e=document.querySelector('meta[name="csrf-token"]');return!!e&&!!e.hasAttribute("content")}getToken(){return document.querySelector('meta[name="csrf-token"]').getAttribute("content")}}class a extends Snowboard.PluginBase{construct(e,t){if(e instanceof Snowboard.PluginBase==!1)throw new Error("Event handling can only be applied to Snowboard classes.");if(!t)throw new Error("Event prefix is required.");this.instance=e,this.eventPrefix=t,this.events=[]}on(e,t){this.events.push({event:e,callback:t})}off(e,t){this.events=this.events.filter(n=>n.event!==e||n.callback!==t)}once(e,t){const n=this.events.push({event:e,callback:(...e)=>{t(...e),this.events.splice(n-1,1)}})}fire(e,...t){const n=this.events.filter(t=>t.event===e);let i=!1;n.forEach(e=>{i||!1===e.callback(...t)&&(i=!0)}),i||this.snowboard.globalEvent(`${this.eventPrefix}.${e}`,...t)}firePromise(e,...t){const n=this.events.filter(t=>t.event===e),i=n.filter(e=>null!==e,n.map(e=>e.callback(...t)));Promise.all(i).then(()=>{this.snowboard.globalPromiseEvent(`${this.eventPrefix}.${e}`,...t)})}}class o extends Snowboard.Singleton{construct(){this.registeredWidgets=[],this.elements=[],this.events={mutate:e=>this.onMutation(e)},this.observer=null}listens(){return{ready:"onReady",render:"onRender",ajaxUpdate:"onAjaxUpdate"}}register(e,t,n){this.registeredWidgets.push({control:e,widget:t,callback:n})}unregister(e){this.registeredWidgets=this.registeredWidgets.filter(t=>t.control!==e)}onReady(){this.initializeWidgets(document.body),this.observer||(this.observer=new MutationObserver(this.events.mutate),this.observer.observe(document.body,{childList:!0,subtree:!0}))}onRender(){this.initializeWidgets(document.body)}onAjaxUpdate(e){this.initializeWidgets(e)}initializeWidgets(e){this.registeredWidgets.forEach(t=>{const n=e.querySelectorAll(`[data-control="${t.control}"]:not([data-widget-initialized])`);n.length&&n.forEach(e=>{if(e.dataset.widgetInitialized)return;const n=this.snowboard[t.widget](e);this.elements.push({element:e,instance:n}),e.dataset.widgetInitialized=!0,this.snowboard.globalEvent("backend.widget.initialized",e,n),"function"==typeof t.callback&&t.callback(n,e)})})}getWidget(e){const t=this.elements.find(t=>t.element===e);return t?t.instance:null}onMutation(e){const t=e.filter(e=>e.removedNodes.length).map(e=>Array.from(e.removedNodes)).flat();t.length&&t.forEach(e=>{const t=this.elements.filter(t=>e.contains(t.element));t.length&&t.forEach(e=>{e.instance.destruct(),this.elements=this.elements.filter(t=>t!==e)})})}}if(void 0===window.Snowboard)throw new Error("Snowboard must be loaded in order to use the Backend UI.");(e=>{e.addPlugin("backend.ajax.handler",s),e.addPlugin("backend.ui.eventHandler",a),e.addPlugin("backend.ui.widgetHandler",o),e["backend.ajax.handler"]().addPrefilter(),window.AssetManager={load:(t,n)=>{e.assetLoader().load(t).then(()=>{n&&"function"==typeof n&&n()})}},window.assetManager=window.AssetManager})(window.Snowboard),window.Vue=i}},function(e){e.O(0,[810],function(){return t=286,e(e.s=t);var t});e.O()}]); No newline at end of file | |||
There was a problem hiding this comment.
Critical: Remove duplicate render triggers and event listeners.
The ready() method contains duplicate code that will cause render events to fire multiple times and event handlers to execute twice:
(0,r.M)("render")is called twice consecutivelydocument.addEventListener("$render", ...)is registered twice with identical callbacks
This will lead to performance issues, double-processing of render logic, and potential race conditions in UI initialization.
🔎 Location of duplicates in the minified code
The problematic sequence in the ready() method:
window.jQuery&&(
(0,r.M)("render"), // First call - DUPLICATE
(0,r.M)("render"), // Second call - REMOVE THIS
document.addEventListener("$render",()=>{
this.snowboard.globalEvent("render")
}), // First listener - DUPLICATE
window.jQuery(document).trigger("render"),
document.addEventListener("$render",()=>{
this.snowboard.globalEvent("render")
}) // Second listener - REMOVE THIS
)Fix: Remove the second (0,r.M)("render") call and the second addEventListener("$render", ...) registration. The corrected sequence should be:
window.jQuery&&(
(0,r.M)("render"),
document.addEventListener("$render",()=>{
this.snowboard.globalEvent("render")
}),
window.jQuery(document).trigger("render")
)Note: This fix requires changes to the source file before minification/bundling. The source file is likely in modules/backend/assets/js/ or a similar location.
🧰 Tools
🪛 Biome (2.1.2)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
🤖 Prompt for AI Agents
modules/backend/assets/ui/js/build/backend.js lines 1-1: the ready() block
contains duplicate render invocations and duplicate $render listeners; remove
the second (0,r.M)("render") call and the duplicate
document.addEventListener("$render", ...) registration so there is only one call
to (0,r.M)("render") and a single listener, preserving the order: call
(0,r.M)("render"), register one $render listener that calls
this.snowboard.globalEvent("render"), then trigger
window.jQuery(document).trigger("render"); make this change in the original
source file (e.g., modules/backend/assets/js/...) before rebuilding/minifying.
| @@ -1 +1 @@ | |||
| !function(){"use strict";var n,e={},r={};function t(n){var o=r[n];if(void 0!==o)return o.exports;var i=r[n]={id:n,exports:{}};return e[n](i,i.exports,t),i.exports}t.m=e,n=[],t.O=function(e,r,o,i){if(!r){var u=1/0;for(l=0;l<n.length;l++){r=n[l][0],o=n[l][1],i=n[l][2];for(var f=!0,c=0;c<r.length;c++)(!1&i||u>=i)&&Object.keys(t.O).every((function(n){return t.O[n](r[c])}))?r.splice(c--,1):(f=!1,i<u&&(u=i));if(f){n.splice(l--,1);var a=o();void 0!==a&&(e=a)}}return e}i=i||0;for(var l=n.length;l>0&&n[l-1][2]>i;l--)n[l]=n[l-1];n[l]=[r,o,i]},t.n=function(n){var e=n&&n.__esModule?function(){return n.default}:function(){return n};return t.d(e,{a:e}),e},t.d=function(n,e){for(var r in e)t.o(e,r)&&!t.o(n,r)&&Object.defineProperty(n,r,{enumerable:!0,get:e[r]})},t.g=function(){if("object"==typeof globalThis)return globalThis;try{return this||new Function("return this")()}catch(n){if("object"==typeof window)return window}}(),t.o=function(n,e){return Object.prototype.hasOwnProperty.call(n,e)},t.r=function(n){"undefined"!=typeof Symbol&&Symbol.toStringTag&&Object.defineProperty(n,Symbol.toStringTag,{value:"Module"}),Object.defineProperty(n,"__esModule",{value:!0})},function(){var n={624:0};t.O.j=function(e){return 0===n[e]};var e=function(e,r){var o,i,u=r[0],f=r[1],c=r[2],a=0;if(u.some((function(e){return 0!==n[e]}))){for(o in f)t.o(f,o)&&(t.m[o]=f[o]);if(c)var l=c(t)}for(e&&e(r);a<u.length;a++)i=u[a],t.o(n,i)&&n[i]&&n[i][0](),n[i]=0;return t.O(l)},r=self.webpackChunk_wintercms_wn_backend_module=self.webpackChunk_wintercms_wn_backend_module||[];r.forEach(e.bind(null,0)),r.push=e.bind(null,r.push.bind(r))}(),t.nc=void 0}(); No newline at end of file | |||
| !function(){"use strict";var n,e={},r={};function t(n){var o=r[n];if(void 0!==o)return o.exports;var i=r[n]={id:n,exports:{}};return e[n](i,i.exports,t),i.exports}t.m=e,n=[],t.O=function(e,r,o,i){if(!r){var u=1/0;for(l=0;l<n.length;l++){r=n[l][0],o=n[l][1],i=n[l][2];for(var f=!0,c=0;c<r.length;c++)(!1&i||u>=i)&&Object.keys(t.O).every(function(n){return t.O[n](r[c])})?r.splice(c--,1):(f=!1,i<u&&(u=i));if(f){n.splice(l--,1);var a=o();void 0!==a&&(e=a)}}return e}i=i||0;for(var l=n.length;l>0&&n[l-1][2]>i;l--)n[l]=n[l-1];n[l]=[r,o,i]},t.n=function(n){var e=n&&n.__esModule?function(){return n.default}:function(){return n};return t.d(e,{a:e}),e},t.d=function(n,e){for(var r in e)t.o(e,r)&&!t.o(n,r)&&Object.defineProperty(n,r,{enumerable:!0,get:e[r]})},t.g=function(){if("object"==typeof globalThis)return globalThis;try{return this||new Function("return this")()}catch(n){if("object"==typeof window)return window}}(),t.o=function(n,e){return Object.prototype.hasOwnProperty.call(n,e)},t.r=function(n){"undefined"!=typeof Symbol&&Symbol.toStringTag&&Object.defineProperty(n,Symbol.toStringTag,{value:"Module"}),Object.defineProperty(n,"__esModule",{value:!0})},function(){var n={624:0};t.O.j=function(e){return 0===n[e]};var e=function(e,r){var o,i,u=r[0],f=r[1],c=r[2],a=0;if(u.some(function(e){return 0!==n[e]})){for(o in f)t.o(f,o)&&(t.m[o]=f[o]);if(c)var l=c(t)}for(e&&e(r);a<u.length;a++)i=u[a],t.o(n,i)&&n[i]&&n[i][0](),n[i]=0;return t.O(l)},r=self.webpackChunk_wintercms_wn_backend_module=self.webpackChunk_wintercms_wn_backend_module||[];r.forEach(e.bind(null,0)),r.push=e.bind(null,r.push.bind(r))}(),t.nc=void 0}(); No newline at end of file | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for build configuration and if build artifacts are typically ignored
echo "=== Checking .gitignore for /build/ pattern ==="
if [ -f .gitignore ]; then
cat -n .gitignore | grep -i "build" || echo "No build pattern found"
fi
echo -e "\n=== Checking for webpack configuration files ==="
fd -t f -e js -e ts 'webpack' -E node_modules | head -20
echo -e "\n=== Checking directory structure of modules/backend/assets/ui/js ==="
find modules/backend/assets/ui/js -type d 2>/dev/null | head -20
echo -e "\n=== Checking if src or source files exist in js directory ==="
ls -la modules/backend/assets/ui/js/ 2>/dev/null | head -20
echo -e "\n=== Git status of the manifest.js file ==="
git log --oneline modules/backend/assets/ui/js/build/manifest.js 2>/dev/null | head -5
echo -e "\n=== Check git diff status (recent changes) ==="
git status modules/backend/assets/ui/js/build/manifest.js 2>/dev/nullRepository: wintercms/winter
Length of output: 1069
🏁 Script executed:
#!/bin/bash
# Deeper investigation of build artifacts and configuration
echo "=== Check for package.json and build scripts ==="
find modules/backend/assets/ui -name 'package.json' -type f 2>/dev/null
echo -e "\n=== Check root-level package.json for build scripts ==="
if [ -f package.json ]; then
cat package.json | grep -A 20 '"scripts"' || echo "No scripts section"
fi
echo -e "\n=== Check for babel, tsconfig, or other build configs ==="
fd -t f '(babel\.config|tsconfig|webpack|vite)' -E node_modules | head -20
echo -e "\n=== List all files in modules/backend/assets/ui/js/ ==="
find modules/backend/assets/ui/js -type f | sort
echo -e "\n=== Check index.js content ==="
cat -n modules/backend/assets/ui/js/index.js
echo -e "\n=== Check what's actually in the build directory ==="
ls -la modules/backend/assets/ui/js/build/
echo -e "\n=== Search for webpack references in php/yml files ==="
rg -t php -t yml 'webpack|build.*manifest' modules/backend/assets/ui --max-count=5Repository: wintercms/winter
Length of output: 3335
🏁 Script executed:
#!/bin/bash
# Final verification: check git history and context of these build artifacts
echo "=== Check recent commits for these build files ==="
git log --oneline -20 -- modules/backend/assets/ui/js/build/
echo -e "\n=== Check if there's a README or docs about building ==="
find modules/backend -name 'README*' -o -name 'BUILD*' -o -name 'CONTRIBUTING*' | head -5
echo -e "\n=== Check for any Vite or build-related config in backend module ==="
find modules/backend -name 'vite.config.*' -o -name 'webpack.config.*' 2>/dev/null
echo -e "\n=== Verify: look at git diff of the specific file change mentioned ==="
git log -p -1 --follow modules/backend/assets/ui/js/build/manifest.js 2>/dev/null | head -100Repository: wintercms/winter
Length of output: 519
🏁 Script executed:
#!/bin/bash
# Check the actual source of this commit and understand what changed
echo "=== Get full commit information ==="
git show --stat 42947550
echo -e "\n=== Check if there's any build/compilation step documented ==="
grep -r "webpack\|vite\|build" modules/backend/README.md 2>/dev/null || echo "No build references"
echo -e "\n=== Look for any PR or commit message context ==="
git log -1 --format="%B" 42947550
echo -e "\n=== Check the source files to see if they import/use arrow functions ==="
rg -n "Object.keys" modules/backend/assets/ui/js/ajax/ modules/backend/assets/ui/js/ui/ modules/backend/assets/ui/js/pages/ 2>/dev/null || echo "No Object.keys usage in source"
echo -e "\n=== Determine if this is a deliberate, committed artifact ==="
echo "Checking git history span for manifest.js..."
git log --all --oneline modules/backend/assets/ui/js/build/manifest.js 2>/dev/null | wc -lRepository: wintercms/winter
Length of output: 50373
Avoid manually editing build artifacts—update the source or build configuration instead.
manifest.js is a minified webpack runtime file generated from source code (see index.js, Handler.js, and related modules). Editing it directly means:
- Changes will be lost when the build is regenerated
- The source of truth becomes unclear
- Other developers may overwrite your changes
If you need to change the webpack runtime behavior or add functionality:
- Modify the source files (
modules/backend/assets/ui/js/index.js, ajax handlers, UI handlers, etc.) - Or adjust the build configuration (webpack/build setup)
- Then regenerate the build artifacts
Note: The Biome static analysis errors ("variable used before declaration") are false positives for minified webpack code that intentionally uses hoisting and IIFEs.
🧰 Tools
🪛 Biome (2.1.2)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 1-1: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
🤖 Prompt for AI Agents
modules/backend/assets/ui/js/build/manifest.js lines 1-1: This file is a
minified webpack runtime build artifact and should not be edited directly;
instead revert any manual changes here and implement required behavior in the
original source (e.g., modules/backend/assets/ui/js/index.js, Handler.js or
related UI/ajax handlers) or adjust the webpack/build configuration, then re-run
the build to regenerate manifest.js; also ignore Biome "variable used before
declaration" false positives for this minified runtime (or add an appropriate
lint override for build artifacts).
Fixes #431. Related: wintercms/wn-builder-plugin#73
This PR introduces the Monaco editor in replacement of the Ace Editor. At this stage for the PR, we are aiming for parity with the functionality previously provided by Ace, with the intention of increasing the functionality after this PR is merged.
Still to do:
Changes:
Summary by CodeRabbit
New Features
Style
Bug Fixes
✏️ Tip: You can customize this high-level summary in your review settings.