#64558 closed defect (bug) (fixed)
CodeMirror: JavaScript linting is incompatible with newer language features
| Reported by: |
|
Owned by: |
|
|---|---|---|---|
| Milestone: | 7.0 | Priority: | normal |
| Severity: | normal | Version: | 4.9.3 |
| Component: | External Libraries | Keywords: | has-patch has-unit-tests |
| Focuses: | javascript | Cc: |
Description (last modified by )
In #42850 (r42547), JSHint was replaced with Esprima due to the former having a GPL-incompatible license. However, the esprima package has not been updated since July 2018. Because it hasn't been updated in so long, it does not support "newer" syntax ECMAScript syntax (since 2020) like dynamic imports, optional chaining, and nullish coalescing:
async function getMod() { const mod = await import( 'https://example.com/module.js' ); // ❌ ------------^ return mod; } const baz = window?.foo?.bar?.baz; // ❌ -------------^ const quux = baz ?? 'baz'; // ❌ ------------^
The Espree project (part of ESLint) is an actively-maintained substitute for Esprima.
As a follow-up to #48456 (r61539) which upgraded CodeMirror to the latest v5 and Esprima (from 4.0.0 to 4.0.1), we can now do a better upgrade by using Espree instead of Esprima.
Change History (16)
This ticket was mentioned in PR #10806 on WordPress/wordpress-develop by @westonruter.
7 weeks ago
#2
- Keywords has-patch has-unit-tests added
@jonsurrell commented on PR #10806:
5 weeks ago
#5
I noticed that the javascript-lint file isn't correctly watched when testing with npm run dev.
This isn't a blocker, but it's always nice when these tasks all work.
Grunt goes into watch mode, and changes to src/js/_enqueues/vendor/codemirror/javascript-lint.js triggers this:
>> File "src/js/_enqueues/vendor/codemirror/javascript-lint.js" changed. Running "clean:dynamic" (clean) task >> 0 paths cleaned. Running "copy:dynamic-js" (copy) task Copied 1 file Running "uglify:dynamic" (uglify) task >> 1 file created 3.59 kB → 759 B Running "_watch" task
That copy/uglify seems redundant and maybe a codemirror watch task isn't set up?
@jonsurrell commented on PR #10806:
5 weeks ago
#6
I was testing this out and wanted to work with some script modules but that errors on import/export syntax. I created https://github.com/westonruter/wordpress-develop/pull/5 to add module support based on .mjs extension.
@westonruter commented on PR #10806:
5 weeks ago
#7
I noticed that the javascript-lint file isn't correctly watched when testing with
npm run dev.
@sirreal I actually have trouble with npm run dev generally. I can't get watch to work reliably with any files. For example, I run it and then I modify src/js/_enqueues/lib/comment-reply.js, and yet in my terminal it just remains indefinitely at:
Running "_watch" task Waiting...
This seems like a larger issue to be investigated.
#8
@
5 weeks ago
Note: espree v9.6.1 is used because it is already installed via @wordpress/scripts => eslint (v8.45.0) => espree (9.6+).
There is a newer version, v11.1.0, however. We should upgrade once we bump the minimum version of Node and/or @wordpress/scripts is updated to use a newer ESLint. Or maybe this is irrelevant and can go ahead and use v11.1.0 now. In any case, we can make that upgrade in a subsequent commit.
This ticket was mentioned in PR #10900 on WordPress/wordpress-develop by @westonruter.
5 weeks ago
#10
This is a follow-up to:
- https://github.com/WordPress/wordpress-develop/pull/10806
- https://github.com/WordPress/wordpress-develop/pull/10778
To address:
- [ ] Fix TypeScript issues with
code-editor.js. - [ ] Address https://github.com/WordPress/wordpress-develop/pull/10806#discussion_r2787837736
Trac ticket: https://core.trac.wordpress.org/ticket/64558
## Use of AI Tools
Using Gemini CLI
@westonruter commented on PR #10900:
4 weeks ago
#11
Note: The coding standards failure is due to an existing JSHint error with fakejshint.js which is now deprecated. The file was unchanged.
@westonruter commented on PR #10900:
3 weeks ago
#12
This was _not_ committed yet.
@westonruter commented on PR #10900:
3 weeks ago
#13
@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:
13 days ago
#14
The typcheck:js script doesn't run on CI, does it? That would be a good follow-up.
@westonruter commented on PR #10900:
13 days ago
#15
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:
12 days ago
#16
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
src/js/_enqueues/vendor/codemirror/javascript-lint.jswhich usesespree(v9.6.1) for parsing and error reporting.jshintandesprimascripts. Theespreemodule is now loaded via a dynamic import on demand by the new javascript lint integration.tools/vendors/codemirror-entry.js.espreeas a script module insrc/wp-includes/script-modules.php.wp-codemirrorregistration to ensureespreeis included in the importmap.wp_get_code_editor_settings()insrc/wp-includes/general-template.phpto use ES11 defaults.esprimaandjshintscript handles as deprecated insrc/wp-includes/script-loader.php.tools/webpack/codemirror.config.js) to bundleespreeas a module.codemirror-entry.jsto use the new localjavascript-lint.js.Previously developed in https://github.com/westonruter/wordpress-develop/pull/4 as a sub-PR off of https://github.com/WordPress/wordpress-develop/pull/10778
Trac ticket: https://core.trac.wordpress.org/ticket/64558