fix: syntax errors in JS example sections #18186

lionralfs · 2022-07-10T15:58:21Z

Summary

Hey everyone!

I've come across this modernization effort for JS code samples and decided to test the feasibility of using ESLint to automate some of the work. However, I've quickly noticed that around 500 code samples in the files/en-us/web/api/ directory contain syntax errors (ESLint reports parsing errors). I then started to fix some of them, which is the state of this PR. Before proceeding to fix the remaining errors I wanted to ask for some maintainer input on whether this is worth pursuing. I imagine fixing everything in one big PR makes reviewing somewhat painful.

I've purposely not commited my ESLint setup, but if you want to try it for yourself, this is how I did it:

npm install --save-dev eslint eslint-plugin-markdown
Modify .eslintrc.js:

module.exports = {
  extends: [
    // "plugin:json/recommended"
    "plugin:markdown/recommended",
  ],
  parserOptions: {
    ecmaVersion: "latest",
    // allows top-level await among other things
    sourceType: "module",
  },
  env: {
    es6: true,
    browser: true,
    node: true,
  },
};

npx eslint "files/en-us/web/api/**/*.md"

Obviously this is not a perfect setup but it already highlights some existing issues with the code samples.

This PR…

Adds a new document
Rewrites (or significantly expands) a document
Fixes a typo, bug, or other error

Josh-Cena · 2022-07-10T16:07:33Z

Ahhh, I'm also looking into eslint-plugin-markdown. Glad someone is on the same page! Yes, please do go ahead—these PRs are easy to review so just keep it at around ~50 files changed. (It would be a problem if we have 100 syntax errors in our code...)

lionralfs · 2022-07-10T16:33:33Z

So you would prefer PRs in batches of 50 changed files? I've just checked and ~380 files have at least one syntax (parsing) error.

Josh-Cena · 2022-07-10T16:36:14Z

Ah, let's spread them across 4 PRs then—about 100 lines of diff per PR would be the best.

lionralfs · 2022-07-10T16:47:12Z

Just FYI: I'm planning to add a few more fixes to this PR before I open a new one

teoli2003 · 2022-07-10T16:47:34Z

Yes, our idea is to have a linter running periodically (or automatically, we'll see). For this to work, we need to run it once (like you do), find the errors, and fix them.

Then we can run it weekly, and the weekly PR will be small enough to be reviewed quickly.

teoli2003 · 2022-07-10T16:48:21Z

Just FYI: I'm planning to add a few more fixes to this PR before I open a new one

You are welcome.

lionralfs · 2022-07-10T17:29:50Z

files/en-us/web/api/xranchor/delete/index.md

@@ -19,7 +19,7 @@ The **`delete()`** method of the {{domxref("XRAnchor")}} interface removes an an
 ## Syntax

 ```js
-delete()
+anchor.delete()


Not necessarily a syntax fix but this stops the parser from complaining since delete is an operator.

Yep—we do the same for generator.return(). See also https://github.com/orgs/mdn/discussions/149

No, we don't update these. We will change the type from js to jssyntax very soon. Meanwhile, we should ignore this one.

Ugh... Maybe until we reach a resolution for https://github.com/orgs/mdn/discussions/148...

I'm not sure whether you agreed on this change or want me to revert it. I'd appreciate an update

Ugh... Maybe until we reach a resolution for https://github.com/orgs/mdn/discussions/148...

Yes, let's keep the original until this discussion is done (and the syntax section in the meta-docs).

lionralfs · 2022-07-10T17:36:39Z

Some things I'm noticing:

Sometimes, the language mode is set incorrectly (html/js). For pseudocode, I've removed it completely
Some variable names are problematic in certain contexts, for example: the parser doesn't like default in ESM contexts. Probably relevant for further work (see comment above regarding automated linting)

teoli2003 · 2022-07-10T17:40:05Z

files/en-us/web/api/window/open/index.md

@@ -120,7 +120,7 @@ function openRequestedPopup() {

 ## Best practices

-```js
+```html
 <script type="text/javascript">


I think we should keep it js and remove the <script> element (3 times in this file)

As those code blocks still include html, would you use a js code block followed by a html one?

Yes, please.

Josh-Cena · 2022-07-10T17:44:55Z

Some variable names are problematic in certain contexts, for example: the parser doesn't like default in ESM contexts.

We should rename those variables.

teoli2003 · 2022-07-10T17:59:15Z

Some variable names are problematic in certain contexts, for example: the parser doesn't like default in ESM contexts.

We should rename those variables.

Yes, variables (or pseudo-variables in syntax sections) that have problematic names should be replaced: people often cut and past these, and then modify (actually declaring the variable with the same name, etc.)

teoli2003

(See comments above)

teoli2003

Thanks a lot. I'm really happy with the idea to use ESLint to find and fix syntax errors.

OnkarRuikar · 2022-07-17T06:58:03Z

I've purposely not commited my ESLint setup..

@lionralfs it would be great if you commit just the setup(no .md files) on a feature branch in your fork(or a PR here) and share with us.

We need to have discussion about which rules to disable. For example, we need to disable no-undef, no-console, no-alert etc.(this will save us from annotating hundreds of code blocks to disable the rules individually).
The idea is when we'll reach zero .md files flagged in content(after fixing them) then we can commit the ESLint setup along with our agreed config file.

Josh-Cena · 2022-07-17T07:01:02Z

We'll start by fixing syntax errors so that ESLint can work at all—and then negotiating with the parser about what to parse and what to report. For example I still strongly believe

```js
const a = 1;
// OR
const a = 2;
```

Is strictly better than two code blocks or using two identifiers, just to make the parser happy. Making this work probably requires tweaking the parser a bit.

Josh-Cena · 2022-07-17T07:05:24Z

Oh btw, I think we can have parserOptions and env right now in the main branch's config—just not the plugins and rules sections. When I'm writing random test scripts I often come across ESLint parser errors like "class is a reserved word" because the parser is malconfigured.

lionralfs · 2022-07-17T10:06:23Z

I've pushed my ESLint setup to my fork: lionralfs:eslint-config
This also includes an .eslintignore file which I've been using to skip over certain .md files with more complicated syntax errors.
I've been documenting what I've skipped in various comments/PR descriptions and will most likely compile them into a single list when I'm done with the remaining errors (which should only be 1 more PR).

lionralfs · 2022-07-17T11:45:20Z

Ok so here we go. PR #18442 is the fifth and last (for now) PR to fix syntax/parsing errors.

As promised, I'll document all encountered issues that I've skipped here:

List of remaining syntax parsing/errors

delete()
- see comments here: fix: syntax errors in JS example sections #18186 (comment)
"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 not the only page where examples are written like this, I'll refer to this style of writing as "chunked" from now on
glsl?
- See comment: fix: syntax errors in JS example sections (v2) #18191 (comment)
return outside of function
return outside of function
return outside of function
js hidden?
js hidden?
js hidden?
js hidden?
js hidden?
"chunked" code
"code diff"
loose case blocks all throughout this file
"chunked" code
method without surrounding class
- multiple times in this file
"chunked" code
method without surrounding class
delete(name)
loose case block
delete(property)
"chunked" code
methods outside of class
{{deprecated_inline}} inside code block
"raw" objects
- These result in Parsing error: Unexpected token :
method outside of class
class as arg name
return outside of function
"raw" objects
- These result in Parsing error: Unexpected token :
"chunked" code
"chunked" code
delete(key)
delete()
continue()
delete(name)
delete(name)
delete(font)
method outside of class
with in strict mode
with in strict mode
with in strict mode
with in strict mode
- See comment fix: syntax errors in JS example sections (v5) #18442 (comment)
delete(value)
methods ouside of class
- All throughout this file
delete(name/options)
delete(cacheName)
delete(request)
return outside of function

The .eslintignore file I've used to exclude all of these

files/en-us/web/api/xranchor/delete/index.md
files/en-us/web/api/webvr_api/using_the_webvr_api/index.md
files/en-us/web/api/webglrenderingcontext/texparameter/index.md
files/en-us/web/api/webgl_api/tutorial/using_textures_in_webgl/index.md
files/en-us/web/api/webgl_api/tutorial/lighting_in_webgl/index.md
files/en-us/web/api/webgl_api/tutorial/creating_3d_objects_using_webgl/index.md
files/en-us/web/api/webgl_api/by_example/textures_from_code/index.md
files/en-us/web/api/webgl_api/by_example/scissor_animation/index.md
files/en-us/web/api/webgl_api/by_example/raining_rectangles/index.md
files/en-us/web/api/webgl_api/by_example/hello_vertex_attributes/index.md
files/en-us/web/api/webgl_api/by_example/hello_glsl/index.md
files/en-us/web/api/web_speech_api/using_the_web_speech_api/index.md
files/en-us/web/api/web_crypto_api/non-cryptographic_uses_of_subtle_crypto/index.md
files/en-us/web/api/web_audio_api/web_audio_spatialization_basics/index.md
files/en-us/web/api/web_audio_api/visualizations_with_web_audio_api/index.md
files/en-us/web/api/web_audio_api/using_audioworklet/index.md
files/en-us/web/api/web_audio_api/simple_synth/index.md
files/en-us/web/api/usbdevice/opened/index.md
files/en-us/web/api/urlsearchparams/delete/index.md
files/en-us/web/api/transformstreamdefaultcontroller/error/index.md
files/en-us/web/api/stylepropertymap/delete/index.md
files/en-us/web/api/streams_api/using_readable_streams/index.md
files/en-us/web/api/shadowroot/index.md
files/en-us/web/api/rtcpeerconnection/getstats/index.md
files/en-us/web/api/readablestreambyobreader/read/index.md
files/en-us/web/api/readablebytestreamcontroller/error/index.md
files/en-us/web/api/paintworklet/registerpaint/index.md
files/en-us/web/api/mediastream_image_capture_api/index.md
files/en-us/web/api/mediadevices/getusermedia/index.md
files/en-us/web/api/media_streams_api/taking_still_photos/index.md
files/en-us/web/api/indexeddb_api/checking_when_a_deadline_is_due/index.md
files/en-us/web/api/idbobjectstore/delete/index.md
files/en-us/web/api/idbcursor/delete/index.md
files/en-us/web/api/idbcursor/continue/index.md
files/en-us/web/api/headers/delete/index.md
files/en-us/web/api/formdata/delete/index.md
files/en-us/web/api/fontfaceset/delete/index.md
files/en-us/web/api/elementinternals/states/index.md
files/en-us/web/api/element/replacewith/index.md
files/en-us/web/api/element/remove/index.md
files/en-us/web/api/element/prepend/index.md
files/en-us/web/api/element/append/index.md
files/en-us/web/api/customstateset/delete/index.md
files/en-us/web/api/css_painting_api/guide/index.md
files/en-us/web/api/cookiestore/delete/index.md
files/en-us/web/api/cachestorage/delete/index.md
files/en-us/web/api/cache/delete/index.md
files/en-us/web/api/abortsignal/abort/index.md

teoli2003 · 2022-07-18T11:08:44Z

Hi! I've created a discussion with the amazing list of errors you found. I grouped them into categories, problems, so that we can discuss them and find solutions: mdn/discussions#158

That way, we will have the discussion in one well-visible place rather than buried in PR.

🎉 Thanks a lot for this amazing work, @lionralfs! Much, much appreciated. 🎉

lionralfs requested a review from a team as a code owner July 10, 2022 15:58

lionralfs requested review from jpmedley and removed request for a team July 10, 2022 15:58

github-actions bot added the Content:WebAPI Web API docs label Jul 10, 2022

lionralfs marked this pull request as draft July 10, 2022 15:59

This comment was marked as resolved.

Sign in to view

teoli2003 approved these changes Jul 10, 2022

View reviewed changes

lionralfs commented Jul 10, 2022

View reviewed changes

teoli2003 reviewed Jul 10, 2022

View reviewed changes

lionralfs marked this pull request as ready for review July 10, 2022 17:44

teoli2003 suggested changes Jul 10, 2022

View reviewed changes

lionralfs added 4 commits July 10, 2022 21:11

fix: syntax errors in JS example sections

bd75693

more syntax fixes

0fba666

rename variable: default -> defaultValue

6c51c13

separate js and html blocks

cb67bcc

lionralfs force-pushed the fix-js-example-syntax-errors branch from 5f0d73c to cb67bcc Compare July 10, 2022 19:11

revert anchor.delete() change

7c6766a

lionralfs requested a review from teoli2003 July 10, 2022 19:40

teoli2003 approved these changes Jul 10, 2022

View reviewed changes

teoli2003 merged commit 5b82af5 into mdn:main Jul 10, 2022

lionralfs deleted the fix-js-example-syntax-errors branch July 10, 2022 19:55

This was referenced Jul 10, 2022

fix: syntax errors in JS example sections (v2) #18191

Merged

fix: syntax errors in JS example sections (v3) #18307

Merged

fix: syntax errors in JS example sections (v4) #18424

Merged

lionralfs mentioned this pull request Jul 17, 2022

fix: syntax errors in JS example sections (v5) #18442

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: syntax errors in JS example sections #18186

fix: syntax errors in JS example sections #18186

lionralfs commented Jul 10, 2022 •

edited

Loading

This comment was marked as resolved.

Josh-Cena commented Jul 10, 2022

lionralfs commented Jul 10, 2022

Josh-Cena commented Jul 10, 2022 •

edited

Loading

lionralfs commented Jul 10, 2022

teoli2003 commented Jul 10, 2022

teoli2003 commented Jul 10, 2022

lionralfs Jul 10, 2022

Josh-Cena Jul 10, 2022

teoli2003 Jul 10, 2022

Josh-Cena Jul 10, 2022

lionralfs Jul 10, 2022

teoli2003 Jul 10, 2022

lionralfs commented Jul 10, 2022

teoli2003 Jul 10, 2022

lionralfs Jul 10, 2022

teoli2003 Jul 10, 2022

Josh-Cena commented Jul 10, 2022

teoli2003 commented Jul 10, 2022

teoli2003 left a comment

teoli2003 left a comment

OnkarRuikar commented Jul 17, 2022 •

edited

Loading

Josh-Cena commented Jul 17, 2022

Josh-Cena commented Jul 17, 2022

lionralfs commented Jul 17, 2022 •

edited

Loading

lionralfs commented Jul 17, 2022 •

edited

Loading

teoli2003 commented Jul 18, 2022

fix: syntax errors in JS example sections #18186

fix: syntax errors in JS example sections #18186

Conversation

lionralfs commented Jul 10, 2022 • edited Loading

Summary

This comment was marked as resolved.

Josh-Cena commented Jul 10, 2022

lionralfs commented Jul 10, 2022

Josh-Cena commented Jul 10, 2022 • edited Loading

lionralfs commented Jul 10, 2022

teoli2003 commented Jul 10, 2022

teoli2003 commented Jul 10, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lionralfs commented Jul 10, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Josh-Cena commented Jul 10, 2022

teoli2003 commented Jul 10, 2022

teoli2003 left a comment

Choose a reason for hiding this comment

teoli2003 left a comment

Choose a reason for hiding this comment

OnkarRuikar commented Jul 17, 2022 • edited Loading

Josh-Cena commented Jul 17, 2022

Josh-Cena commented Jul 17, 2022

lionralfs commented Jul 17, 2022 • edited Loading

lionralfs commented Jul 17, 2022 • edited Loading

teoli2003 commented Jul 18, 2022

lionralfs commented Jul 10, 2022 •

edited

Loading

Josh-Cena commented Jul 10, 2022 •

edited

Loading

OnkarRuikar commented Jul 17, 2022 •

edited

Loading

lionralfs commented Jul 17, 2022 •

edited

Loading

lionralfs commented Jul 17, 2022 •

edited

Loading