How to fix (non-trivial) ESLint problems in MDN code snippets

[teoli2003]

Hi all!

We have a mid-term goal to use more linters (like Markdownlint, ESLint, or Prettier) on MDN. We would like to use ESLint, a linter for code samples and snippets.

This brings several advantages for our readership:

• Fewer syntax errors: ESLint found a lot of missing parenthesis, braces, semi-colons, and other syntax errors.
• Uniform code formatting makes it easier to read MDN; ESLint code format is very common.
• Easier cut & paste: as ESLint validates the code and there are fewer errors, it is easier to copy the code and get it to work. We know that our readers do this a lot.

If we want to run ESLint automatically, we need to clean MDN first, or each PR will scream from detected problems. Even if we end up running ESLint periodically, as we do with Markdownlint, we will have so many potential errors raised that the tool will become useless. In addition, getting better acquainted with this tool is useful: getting to the hassle of setting up the toolchain to hit one problem after the other is not a good methodology.

@lionralfs ran ESLint on web/javascript and web/audio and created PRs to fix them. 🎉 This is amazing, and they solved a lot of errors. See the (large) PRs: mdn/content#18186, mdn/content#18191, mdn/content#18307, mdn/content#18424, and mdn/content#18442.

This great work uncovered a few problems. We need to solve them if we want to use ESLint automatically. Here is the list of the remaining problems, grouped by category. Note: The id of each group is fixed, so we can refer to it in the discussion that will follow.

Goal of this discussion: We should discuss potential solutions for each of these problems. I will update this original post with outcomes or potential outcomes. Once we know what we need to do, hopefully, we will be able to move forward in setting up an ESLint for our examples.

Problems to solve

Note: Problems IDs are permanent; you can refer to an issue as PR3, PR12, …

👍 = we have a solution for this problem
✅ = solution implemented

[PR1] Code ellipsis. 👍

• The syntactically incorrect ... is used to replace missing code.
• We are dealing with these by using comments //... or /* ... */ and a few variations depending on the language
• Note: This won't work for JSON as comments are illegal there (but this is outside the scope for the moment), and we could allow jsonc, an extension of JSON with comments.
• What is missing: a PR to update the meta-documentation to explain how to mark ellipses in code samples.

[PR2] Pseudo-code in syntax boxes. 👍

• delete() delete(name) delete(property) delete(key) delete() continue() delete(name) delete(name) delete(font) delete(value) delete(name/options) delete(cacheName) delete(request) class as arg name
• discussion
• Syntax boxes use pseudo-js, not real js. We still want to highlight them as js, but we want to tell ESLint to ignore them.
• We create a Prism alias, jssyntax, that we'll apply on syntax boxes and tell ESLint to ignore them.
• PR to create the alias: Add jssyntax as Prism alias for syntax boxes yari#6682

[PR3] Chunked block of code.

• "chunked" code "chunked" code "chunked" code "chunked" code "chunked" code "chunked" code "chunked" code
• The code sections/examples on this page are written in a specific way where a large block of code is intertwined with text paragraphs, obviously syntax-breaking. This is a perfectly legitimate way of presenting content.
• Note: these chunked snippets will likely have an impact on Prettier if we want to use it as some point (with the indentation, for example)

[PR4] GLSL block of code 👍

• glsl
• Discussion
• GLSL (OpenGL Shading Language) is a shading language used in WebGL shaders. We should use the right Prism highlighter. We use it on reference pages and in any guide page about shaders.
• A PR to fix mdn/yari to allow it should be created.

[PR5] Return outside of a function

• return return return return return
• These are chunk of code that are assumed to be inside a function but the function is not declared.

[PR6] Method without surrounding class

• method (multiple occurences) method method method methods (several)
• It looks like a kind of chunked code: code of a method is described, with it being in a code snippet, obviously outside the classstructure (that is not necessary defined earlier).

[PR7] with used in strict mode

• with with with in strict mode with
• ESLint was configured to run in "module" mode, which implies strict mode. Module mode is practical as it allows top-level async/await, so not running it in that mode leads to other problems; strict mode is good: we want our code to be ready for strict mode. Obviously, when we speak about with, we want to avoid linting in strict mode.

[PR8] JS + hidden to complete live examples

• js hidden? js hidden? js hidden? js hidden? js hidden?
• This are special chunked code use to make the live example work. (They are rarely used that way on MDN)

[PR9] Loose case blocks

• loose case blocks(all throughout this file), loose case block
• The first one is a special case of "chunked" content, and the second occurrence is a poor example that we should improve.

[PR10] "raw" objects

• "raw" objects "raw" objects
• These result in Parsing error: Unexpected token :
• They are demonstrations of objects, often used as dictionaries (that will be used as arguments for an options parameter or as return value). When used out of these contexts, they are syntactically incorrect.

[PR11] Macro added to code ✅

• {{deprecated_inline}} inside code block
• One occurrence, we should put a comment instead (or revise the whole section).
• Fixed: Fix syntax errors content#18475

[PR12] Code diffs 👍

• "code diff"
• It is not JS but a diff
• PR to fix this in content: Use diff instead of js to mark a diff block content#18477
• PR to add diff to prism in mdn/yari needs to created.

cc/ @lionralfs, @OnkarRuikar, @Josh-Cena, @Rumyra, @schalkneethling, @wbamberg, @Elchi3

[Josh-Cena]

Pseudo-code in syntax boxes

The various delete usage is related to https://github.com/orgs/mdn/discussions/148. I still strongly believe adding a subject in the syntax box reflects how people use it in real life.

Chunked block of code.

That doesn't look good—should be transformed to comments if possible. Otherwise we may need to invent some kind of special layout in Yari so we can display the text as a floating box or a two-column layout.

Return outside of a function

Worse noting that at least one parser (probably Babel?) has a globalReturn option, which will make this valid. (This option exists because CommonJS modules are put into a function body, so return statements are valid at the top scope in CJS.)

with used in strict mode

We should definitely parse most examples as scripts.

---

Also, several general ideas:

• eslint-disable comments exist. We can always slap these comments over code blocks we think are not worth linting. (e.g. "raw objects")
• We should look into the parser options and see how we can make it more tolerant to different code fragments and try its best to parse valid structures.

[lionralfs]

Worse noting that at least one parser (probably Babel?) has a globalReturn option, which will make this valid. (This option exists because CommonJS modules are put into a function body, so return statements are valid at the top scope in CJS.)

I wanna point out that this doesn't work when parsing as module instead of script. See eslint/eslint#2636

[Josh-Cena]

Yeah, because CJS is script as well... I think we should just fix the top level return considering we aren't actually writing CJS anyway.

[OnkarRuikar]

How about at first we put all the files in ignore list and remove them as the PRs fix them? This way we'll get the ball rolling and the entire effort won't stop because of few un-fixable files. 🙂

---

[PR3] Chunked block of code

Fixing all the blocks will be a huge effort. And they are part of live samples as well.
We need to check how many of them can be safely ignored using inline configuration comments:

<!-- eslint-skip -->

```js
// desired troublesome code
```

We don't have to ignore each block. If it's too much we can ignore entire sections or even entire pages. We can put them in .eslintignore file as well.

[PR5] [PR6] [PR9]

If there are few cases then we can disable linting/rule locally.

[PR7] with used in strict mode

There shouldn't be many withs. We can disable linting/rule locally.

[PR8] JS + hidden to complete live examples

Until a permanent solution is found we simply need to skip these using <!-- eslint-skip -->.

[PR10] "raw" objects

We can mark them as jssyntax.

[PR11] Macro added to code

Those in meta pages describing kuma macros shouldn't be marked as js. We can use jssyntax for them.

Vue and Ember pages are full of these but only few occurrences. We can mark them as jssyntax or disable linting on entire pages.

---

The linting can be done without some rules to keep it simple for contributors and keep alive for longer.

Some of the rules that we can disable globally:
no-undef
no-unused-expressions
no-unused-vars
padded-blocks
no-alert
no-console
etc.

We do not have to enforce all the rules available in ESLint right away. After the syntax errors are fixed, we can tighten the grip few rules at a time depending on how fast we fix the lint rule in content.

[teoli2003]

I would like first to try to avoid any local disabling: it adds complexity to the writers that have to know about this.

I'm not completely against it, but we should try to find semantic and logical solutions, before resorting to this as a last resort.

For example, I'm against using jssyntax for [PR10] as this goes against the semantic of jssyntax that is to mark syntax box and not any js code that is out of context.

[teoli2003]

[PR11] It looks to me that the example you gave in the meta-docs are not JS, it is text that we add in our Markdown, and we should just not highlight them as JavaScript.

[OnkarRuikar]

I would like first to try to avoid any local disabling: it adds complexity to the writers that have to know about this.

I agree. In the end it'll fall upon reviewers and will stretch the reviews. The linting is going to be hard to fit in to PR workflows…

I'm against using jssyntax for [PR10] as this goes against the semantic of jssyntax

We need to have something like jslike: an easy escape hatch to avoid linting for those who don't know about the configuration comments mechanism. There isn't any so I resorted to jssyntax in the suggestion 😅

[PR11] It looks to me that the example you gave in the meta-docs are not JS

The PR handling this mdn/content#18493

[lionralfs]

How about at first we put all the files in ignore list and remove them as the PRs fix them? This way we'll get the ball rolling and the entire effort won't stop because of few un-fixable files. 🙂

I absolutely agree. Any solution that prevents new syntax errors from being introduced would be a big win, either as a PR check or a daily job of some sort. I've already spotted newly added errors since fixing.

[teoli2003]

[PR8] js + hidden

These blocks are not displayed to users. Can we just skip any blocks that have ```js hidden?

(and similarly ```js example-bad )

[teoli2003]

I would say that hidden should be valid syntax, but not alone: they are used for our live samples, and these will break if they are not valid. In some cases, they are additions to make the whole JS of the section a complete JS for the live sample system (like here), and are invalid taken alone.

[OnkarRuikar]

Identifying if it's the only JS block for a live sample won't be easy cos live samples have weird code collection scope.

Changing live sample mechanism itself will require updates to ~2,455 EmbedLiveSamples.

---

Just thinking, if we could tweak the markdown eslinter so that it'll automatically wrap syntax error causing blocks in <!-- eslint-skip --> comments. Reviewers/contributors will fix only the once that can be fixed (and remove the skip comment). This way linter won't block anyone. The blocks that do not have syntax errors will be linted for all other rules like no-var etc.

[teoli2003]

I'm really not keen on HTML comments. My personal experience is that they are very complex and costly to maintain.

[OnkarRuikar]

I'm really not keen on HTML comments.

Can we use skip-lint to tag code blocks which we do not want to lint?

```js hidden skip-lint
(function(){
```

[teoli2003]

This would be better. Nevertheless, this still puts the burden on the editors: they need to understand our toolchain well to contribute. Also, it adds a link between our toolchain and the content. If there weren't a linter, there wouldn't be a directive.

That's why I would prefer a semantic solution that would be logical and natural in the document.

[rubiesonthesky]

I have one new case, "This number literal will lose precision at runtime" https://eslint.org/docs/latest/rules/no-loss-of-precision

I don't know if it's trivial fix or not but this is found in pages about Audio.

• https://developer.mozilla.org/en-US/docs/Web/API/ConstantSourceNode
• https://developer.mozilla.org/en-US/docs/Web/API/Web_Audio_API/Simple_synth

[OnkarRuikar]

There is some friction between exiting markdownlint and Prettier. They format some things differently. So they'll keep correcting/complaining each other.

If we can not find the common ground between them then, I am just thinking, if possible we could use use Prettier only for embedded languages. And let markdownlint continue linting markdown content. For the migration we are able to run Prettier on a particular language in isolation.