#64661 closed defect (bug) (fixed)
Code Editor: Linting runs twice at initialization
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 7.0 | Priority: | normal |
| Severity: | normal | Version: | 4.9 |
| Component: | General | Keywords: | has-patch |
| Focuses: | javascript | Cc: |
Description
As reported by @jonsurrell:
I added some logging here (
console.log( 'Parsing with: %o', getEspreeOptions( options ) )) to understand the options and I noticed something (note this is with https://github.com/westonruter/wordpress-develop/pull/5 so addsmoduleconfig).
The plugin linting seems to be triggered twice, once with what appear to be defaults and again with the expected config. This creates a race, where sometimes on load this is printed:
Parsing with: {ecmaVersion: 'latest', sourceType: 'script', ecmaFeatures: {…}} Parsing with: {ecmaVersion: 11, sourceType: 'module', ecmaFeatures: {…}}And the lint is performed as expected. However, sometimes this is the order and the default linting is applied:
Parsing with: {ecmaVersion: 11, sourceType: 'module', ecmaFeatures: {…}} Parsing with: {ecmaVersion: 'latest', sourceType: 'script', ecmaFeatures: {…}}This does seem to be happening before this PR, but it seems very consistent. I'm always seeing it run lint with the desired options second.
This is a follow-up to the work to upgrade CodeMirror to the latest v5 (#48456) and to using Espree instead of Espree for JS linting (#64558).
Change History (10)
This ticket was mentioned in PR #10900 on WordPress/wordpress-develop by @westonruter.
3 weeks ago
#1
#2
@
3 weeks ago
- Owner set to westonruter
- Status changed from new to accepted
This is closely related to #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.
@westonruter commented on PR #10900:
3 weeks ago
#4
This was _not_ committed yet.
@westonruter commented on PR #10900:
2 weeks ago
#5
@sirreal:
I've left some feedback mostly around the TypeScript setup. If you'd like, I'm happy to work on adjustments and push changes.
Please do! Please amend as you see fit. I'm definitely not a TypeScript expert, so your expertise is much appreciated.
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.
That sounds good to me. I think in this PR that would mean we could turn src/js/_enqueues/lib/codemirror/javascript-lint.js and tools/vendors/codemirror-entry.js into TypeScript files. The other JS files, namely src/js/_enqueues/lib/codemirror/htmlhint-kses.js and src/js/_enqueues/wp/code-editor.js I think should remain as JS since they should be able to be run directly in the browser. (For example, I symlink them to src/wp-includes to avoid having to run a build when I make changes, although I'm willing to concede if this is just me.)
Whether a file is TypeScript or not could also serve as the condition for whether to check types.
There is a lot of type information added to code-editor.js though, so we should be sure to add that as an allow-list of JS files that we also start type-checking. I really would like to start incrementally adding more and more of the core JS files to beying type-checked as well.
@jonsurrell commented on PR #10900:
9 days ago
#6
The typcheck:js script doesn't run on CI, does it? That would be a good follow-up.
@westonruter commented on PR #10900:
9 days ago
#7
Thank you!
The
typcheck:jsscript doesn't run on CI, does it? That would be a good follow-up.
Yes, that is what Core-64662 for.
@westonruter commented on PR #10900:
9 days ago
#9
The
typcheck:jsscript doesn't run on CI, does it? That would be a good follow-up.
Ready for review: https://github.com/WordPress/wordpress-develop/pull/11131
This is a follow-up to:
To address:
code-editor.js.In subsequent PRs:
precommittask and GHA workflow.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 resolves logical and build-process issues.
#### 1. TypeScript Static Analysis & Type Safety
tsconfig.jsonconfigured for JS checking (checkJs: true) and strict mode (strict: true).typings/globals.d.tsto define global interfaces forwp,jQuery,_,Backbone, andHTMLHint.code-editor.js(CodeEditorInstance,LintingController,CombinedLintOptions, etc.) to replace genericObjectandFunctiontypes.anyusage in favor of specific intersection types and unions.#### 2. Code Modernization & Refactoring
code-editor.js:constandletvariable declarations.event.keyCodewithevent.keyandsubstr()withslice().Element.classList,Node.contains).htmlhint-kses.js:for...ofloops).#### 3. Build Process & Reorganization
fakejshint.js,htmlhint-kses.js,javascript-lint.js) fromvendor/tolib/to distinguish them from third-party vendor code.esprima.jssource file; the build now correctly sources the latest version from the npm package.options.optionsfor linting, following modern CodeMirror standards..eslintrc.jsoninlib/codemirror/to correctly signal ES module parsing forjavascript-lint.jswhile maintaining compatibility for other files.#### 4. Dependencies
@types/jquery,@types/codemirror,@types/underscore,@types/espree, and@types/htmlhintas devDependencies to support the static analysis effort.All changes have been verified via
tscandjshint.## Fixed PhpStorm Inspections
## Use of AI Tools
I used Gemini CLI for granular commits, each of which I reviewed.