Code Editor: Improve types and options handling#10900
Code Editor: Improve types and options handling#10900westonruter wants to merge 80 commits intoWordPress:trunkfrom
Conversation
This update replaces all 'var' declarations with 'const' or 'let' and adds the 'eslint-env es6' directive to ensure proper parsing by ESLint. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Updates the JSDoc types from {CodeMirror} to {CodeMirror.Editor} for editor instances to correctly reference the instance interface instead of the namespace. This resolves typing issues in IDEs where methods like getOption() were reported as unresolved.
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Replaces the deprecated String.prototype.substr() method with slice() in code-editor.js to adhere to modern JavaScript best practices and resolve editor warnings. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Updates the keyboard event handling in code-editor.js to use the modern event.key property instead of the deprecated event.keyCode, following current web standards. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
Resolves "Referenced UMD global variable" warnings in PhpStorm by passing window._ as a parameter to the IIFE and explicitly typing it with JSDoc. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Fleshes out the CodeEditorSettings type definition with properties for CodeMirror, CSSLint, JSHint, and HTMLHint based on the provided configuration object. Updates all settings parameter references to use this new type for improved editor intelligence. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Resolves type mismatch errors in PhpStorm by marking nested properties in the CodeEditorSettings typedef as optional. This allows empty objects to be assigned to these properties in defaultSettings while maintaining autocompletion support. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Updates CodeEditorInstance to use CodeMirror.EditorFromTextArea and explicitly types local variables in wp.codeEditor.initialize to resolve assignability warnings in PhpStorm. Switched to a deep extend for instanceSettings to ensure nested configuration objects are correctly handled. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Updates the CodeEditorSettings typedef to use the official CodeMirror.EditorConfiguration type for the codemirror property and adds specific string literal types for direction and inputStyle. This resolves type assignability errors in PhpStorm. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Replaces the internal editor.display.wrapper property with the public editor.getWrapperElement() method to resolve unresolved variable warnings in IDEs. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Replaces the jQuery.contains() call with the native DOM element.contains() method. This resolves a parameter mismatch error in some IDEs and leverages modern native APIs. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Replaces the jQuery.hasClass() call with the native classList.contains() method. This follows modern JavaScript practices and removes an unnecessary jQuery dependency for this check. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Replaces the invalid onUpdateLintingOverridden.apply() call with a direct function call. The previous usage incorrectly attempted to pass three arguments to apply() and used the annotations array as the 'this' context. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Updates the JSDoc types for editor parameters in configureLinting and configureTabbing to use CodeMirror.EditorFromTextArea. This resolves unresolved method warnings in PhpStorm for getTextArea() and ensures correct typing for instances created via fromTextArea(). Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Updates JSDoc types to include optional performLint and showHint methods provided by CodeMirror addons. Adds a check for performLint existence before calling it to ensure safety and resolve IDE warnings. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Replaces _.filter() with the native Array.prototype.filter() method. This helps PhpStorm correctly infer the type of the annotation object from the LintAnnotation[] array, resolving issues where it was incorrectly identified as a string. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Defines CodeMirrorState and CodeMirrorTokenState typedefs to document properties added by addons (completionActive) and mode-specific states (htmlState, curState). Applies these types to the CodeMirror instance and token variable to resolve unresolved variable warnings in PhpStorm. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Refines the CodeMirrorEditor typedef as a standalone interface with relaxed string signatures for getOption, setOption, on, and off, resolving string assignability warnings in PhpStorm. - Introduces CodeMirrorSettings to correctly extend EditorConfiguration with addon-specific properties. - Updates JSDoc for IIFE parameters and internal functions to use these more precise types. - Adds the 'focused' property to CodeMirrorState. - Explicitly types the CodeMirror instance in wp.codeEditor.initialize. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Initializes shouldAutocomplete to false and uses boolean coercion for the HTML tag name check. This makes the code more robust and ensures the variable consistent with its intended purpose as a boolean flag. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
… JSDoc. Add a 'static-analysis:js' script to package.json that runs 'tsc'. Configure 'tsconfig.json' to check JSDoc in 'src/js/_enqueues/wp/code-editor.js' without emitting files, with settings mirrored from '.jshintrc'. Add global type definitions in 'typings/globals.d.ts' for 'wp', 'jQuery', '_', and 'Backbone'. Update 'src/js/_enqueues/wp/code-editor.js' to use global variables directly, satisfying both TypeScript and JSHint. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Break down CodeEditorSettings into nested typedefs (CSSLintRules, JSHintRules, HTMLHintRules) to resolve parsing errors with dot notation in property names. Rename 'wp.codeEditor~CodeEditorInstance' to 'CodeEditorInstance' to resolve syntax errors caused by the '~' character. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
…ings. - Prefix unused variables '_cm' and '_editor' with an underscore to comply with 'noUnusedLocals' and 'noUnusedParameters'. - Remove the unused 'CodeMirrorState' typedef. - Add 'options' property to the 'CodeMirrorEditor' typedef to allow type-safe access to 'codemirror.options.mode'. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Replace 'any' with 'string[]' in the 'gutters' cast for better specificity. - Use a single direct cast for 'gutters' at declaration. - Change 'unknown' to 'string' in the 'option' cast for improved readability. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Type 'event' as 'JQuery.MouseDownEvent' to resolve IDE warnings. - Cast 'event.target' to 'Node' and 'HTMLElement' for safe property access. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Resolve IDE and TypeScript assignment mismatch errors by explicitly casting the returned editor instance to the custom 'CodeMirrorEditor' type. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
- Integrate CodeMirror 'lint' and 'show-hint' addon types via triple-slash references in 'typings/globals.d.ts'. - Define 'CodeMirrorState' and refine 'CodeMirrorEditor' typedefs in 'code-editor.js' to provide better IDE resolution for 'completionActive', 'performLint', and 'showHint'. - Add a type cast in 'getLintOptions' for safe property access on the lint options object. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-cli <218195315+gemini-cli@users.noreply.github.com>
sirreal
left a comment
There was a problem hiding this comment.
I've left some feedback mostly around the TypeScript setup. If you'd like, I'm happy to work on adjustments and push changes.
Frankly, I'd like to start using TypeScript syntax directly for these files that are already being compiled/bundled where we're adding a lot of type information. Whether a file is TypeScript or not could also serve as the condition for whether to check types.
There was a problem hiding this comment.
Here's the file diff between fdafcd3
and 5f54c0b. No obvious issues in the diff 👍
diff --git o/src/js/_enqueues/vendor/codemirror/htmlhint-kses.js w/src/js/_enqueues/lib/codemirror/htmlhint-kses.js
index 1aa7ffbd08..e08c4c0f5b 100644
--- o/src/js/_enqueues/vendor/codemirror/htmlhint-kses.js
+++ w/src/js/_enqueues/lib/codemirror/htmlhint-kses.js
@@ -1,30 +1,47 @@
/* global HTMLHint */
-/* eslint no-magic-numbers: ["error", { "ignore": [0, 1] }] */
-HTMLHint.addRule({
+/* eslint no-magic-numbers: ["error", { "ignore": [1] }] */
+HTMLHint.addRule( {
id: 'kses',
description: 'Element or attribute cannot be used.',
- init: function( parser, reporter, options ) {
+
+ /**
+ * Initialize.
+ *
+ * @this {import('htmlhint/types').Rule}
+ * @param {import('htmlhint').HTMLParser} parser - Parser.
+ * @param {import('htmlhint').Reporter} reporter - Reporter.
+ * @param {Record<string, Record<string, boolean>>} options - KSES options.
+ * @return {void}
+ */
+ init: function ( parser, reporter, options ) {
'use strict';
- var self = this;
- parser.addListener( 'tagstart', function( event ) {
- var attr, col, attrName, allowedAttributes, i, len, tagName;
-
- tagName = event.tagName.toLowerCase();
+ parser.addListener( 'tagstart', ( event ) => {
+ const tagName = event.tagName.toLowerCase();
if ( ! options[ tagName ] ) {
- reporter.error( 'Tag <' + event.tagName + '> is not allowed.', event.line, event.col, self, event.raw );
+ reporter.error(
+ `Tag <${ event.tagName }> is not allowed.`,
+ event.line,
+ event.col,
+ this,
+ event.raw
+ );
return;
}
- allowedAttributes = options[ tagName ];
- col = event.col + event.tagName.length + 1;
- for ( i = 0, len = event.attrs.length; i < len; i++ ) {
- attr = event.attrs[ i ];
- attrName = attr.name.toLowerCase();
- if ( ! allowedAttributes[ attrName ] ) {
- reporter.error( 'Tag attribute [' + attr.raw + ' ] is not allowed.', event.line, col + attr.index, self, attr.raw );
+ const allowedAttributes = options[ tagName ];
+ const column = event.col + event.tagName.length + 1;
+ for ( const attribute of event.attrs ) {
+ if ( ! allowedAttributes[ attribute.name.toLowerCase() ] ) {
+ reporter.error(
+ `Tag attribute [${ attribute.raw }] is not allowed.`,
+ event.line,
+ column + attribute.index,
+ this,
+ attribute.raw
+ );
}
}
- });
- }
-});
+ } );
+ },
+} );
Please do! Please amend as you see fit. I'm definitely not a TypeScript expert, so your expertise is much appreciated.
That sounds good to me. I think in this PR that would mean we could turn
There is a lot of type information added to |
…ion. Confirmed that top-level 'declare var' in a global script automatically adds variables to the 'Window' interface, making the explicit 'interface Window' augmentation unnecessary. Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com> Co-authored-by: Jon Surrell <jonsurrell@git.wordpress.org>
This reverts commit 62eec88.
sirreal
left a comment
There was a problem hiding this comment.
I've pushed several smaller changes mostly to adjust the TypeScript project configuration.
This seems ready. Thanks!
|
The |
|
Thank you!
Yes, that is what Core-64662 for. |
src/js/_enqueues/wp/code-editor.js
Outdated
| * @since 4.9.0 | ||
| * | ||
| * @param {string|jQuery|Element} textarea - The HTML id, jQuery object, or DOM Element for the textarea that is used for the editor. | ||
| * @param {string|Element} textarea - The HTML id, jQuery object, or DOM Element for the textarea that is used for the editor. |
There was a problem hiding this comment.
@sirreal This change seems like it should be undone. In fact, a jQuery object is expected to be able to be passed in here:
There was a problem hiding this comment.
That's fine. This was flagged as an error for me when I tried tsgo, but the stable tsc does not complain.
There was a problem hiding this comment.
I pushed ed9f7b0 that I believe corrects the jQuery types and this type.
The wp.codeEditor.initialize() function is currently passed a jQuery object in core. This reverts commit 9338964.
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
…inting at initialization. * Refactor how CodeMirror is initialized so that the full settings are provided up-front. This avoids the linting from being applied twice at initialization, the first time with an incorrect configuration. * Add initial TypeScript configuration for core with `npm run typecheck:js`. * Add comprehensive types for code editor files: `code-editor.js`, `javascript-lint.js`, and `htmlhint-kses.js`. * Move code editor scripts from `src/js/_enqueues/vendor/codemirror/` to `src/js/_enqueues/lib/codemirror/`. The CodeMirror library is sourced from the npm package as of r61539. * Remove (deprecated) `esprima.js` from being committed to SVN since in r61539 it was switched to using the npm package as its source. * Move `fakejshint.js` to `src/js/_enqueues/deprecated`. Developed in #10900 Follow up to r61611, r61539. Props westonruter, jonsurrell, justlevine. See #64662, #48456. Fixes #64661. git-svn-id: https://develop.svn.wordpress.org/trunk@61800 602fd350-edb4-49c9-b593-d223f7449a82
…inting at initialization. * Refactor how CodeMirror is initialized so that the full settings are provided up-front. This avoids the linting from being applied twice at initialization, the first time with an incorrect configuration. * Add initial TypeScript configuration for core with `npm run typecheck:js`. * Add comprehensive types for code editor files: `code-editor.js`, `javascript-lint.js`, and `htmlhint-kses.js`. * Move code editor scripts from `src/js/_enqueues/vendor/codemirror/` to `src/js/_enqueues/lib/codemirror/`. The CodeMirror library is sourced from the npm package as of r61539. * Remove (deprecated) `esprima.js` from being committed to SVN since in r61539 it was switched to using the npm package as its source. * Move `fakejshint.js` to `src/js/_enqueues/deprecated`. Developed in WordPress/wordpress-develop#10900 Follow up to r61611, r61539. Props westonruter, jonsurrell, justlevine. See #64662, #48456. Fixes #64661. Built from https://develop.svn.wordpress.org/trunk@61800 git-svn-id: http://core.svn.wordpress.org/trunk@61106 1a063a9b-81f0-0310-95a4-ce76da25c4cd
Ready for review: #11131 |
This is a follow-up to:
To address:
code-editor.js.This is closely related to Core-61175 which introduces PHPStan as the PR for this ticket introduces TypeScript, although currently constrained to apply to files related to the code editor. The pull request introduces TypeScript to help guide the refactor to how linting options are gathered so as to avoid the double-linting issue. Instead of initializing with the default options first and then supplying the WordPress options, now it gathers the WordPress options and supplies them into the code editor when it first initializes.
Additionally, the source files were re-organized to remove our CodeMirror extension code from confusingly being located in a
vendordirectory.In subsequent PRs:
precommittask and GHA workflow. See Core-64662.Trac ticket: https://core.trac.wordpress.org/ticket/64661
Gemini Summary
This branch introduces comprehensive TypeScript static analysis for JavaScript files using JSDoc, modernizes several Code Editor components, and refines the build and organizational structure of CodeMirror extensions.
1. TypeScript Static Analysis & Type Safety
tsconfig.jsonconfigured for JS checking (checkJs: true) and strict mode (strict: true). Added atypecheck:jsscript topackage.jsonto facilitate this analysis.typings/globals.d.tsto provide type definitions for WordPress globals (wp),jQuery,_(Underscore),Backbone, andHTMLHint.code-editor.js(e.g.,CodeEditorInstance,LintingController) to replace generic types.anyusage in favor of specific intersection types and unions.2. Code Modernization & Refactoring
code-editor.js:constandlet.event.keyCodewithevent.keyandsubstr()withslice().classList,Node.contains).htmlhint-kses.js:for...ofloops).3. Organization & Build Process
htmlhint-kses.js,javascript-lint.js) fromvendor/tolib/to distinguish them from third-party code.fakejshint.jsfile has been documented as deprecated and moved tosrc/js/_enqueues/deprecated/.esprima.jssource file; the build now correctly sources it from thenpmpackage.options.optionsfor linting, following modern CodeMirror standards..eslintrc.jsoninlib/codemirror/to correctly signal ES module parsing forjavascript-lint.js.4. Dependencies
@typespackages forjquery,codemirror,underscore,espree, andhtmlhintas devDependencies to support the static analysis effort.All changes have been verified via
npm run typecheck:jsandnpx grunt jshint:core.Fixed PhpStorm Inspections
Use of AI Tools
I used Gemini CLI for granular commits, each of which I reviewed.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.