Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

docs: add playground links to correct and incorrect code blocks #17306

Merged
merged 7 commits into from Jul 14, 2023
Merged

docs: add playground links to correct and incorrect code blocks #17306

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
9ec7284
docs: add playground links to correct and incorrect code blocks
JoshuaKGoldberg Jun 23, 2023
09d5bef
Added info options and used existing styles
JoshuaKGoldberg Jul 7, 2023
911656e
Switch options to parserOptions
JoshuaKGoldberg Jul 7, 2023
077ca1e
Use context-based prefix for playground URLs
JoshuaKGoldberg Jul 8, 2023
2a08889
Tweaked styles for more padding, and overlaid vertically in thin screens
JoshuaKGoldberg Jul 12, 2023
91d2801
Reset md files to main
JoshuaKGoldberg Jul 12, 2023
538ec30
Removed icon and increased bottom padding
JoshuaKGoldberg Jul 12, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
53 changes: 51 additions & 2 deletions docs/.eleventy.js
View file
Open in desktop
Expand Up @@ -179,14 +179,63 @@ module.exports = function(eleventyConfig) {
`.trim();
}

/**
* Encodes text in the base 64 format used in playground URL params.
* @param {string} text Text to be encoded to base 64.
* @see https://github.com/eslint/eslint.org/blob/1b2f2aabeac2955a076d61788da8a0008bca6fb6/src/playground/utils/unicode.js
* @returns {string} The base 64 encoded equivalent of the text.
*/
function encodeToBase64(text) {
/* global btoa -- It does exist, and is what the playground uses. */
return btoa(unescape(encodeURIComponent(text)));
}

/**
* Creates markdownItContainer settings for a playground-linked codeblock.
* @param {string} name Plugin name and class name to add to the code block.
* @returns {[string, object]} Plugin name and options for markdown-it.
*/
function withPlaygroundRender(name) {
return [
name,
{
render(tokens, index) {
if (tokens[index].nesting !== 1) {
return "</div>";
}

// See https://github.com/eslint/eslint.org/blob/ac38ab41f99b89a8798d374f74e2cce01171be8b/src/playground/App.js#L44
const parserOptions = tokens[index].info?.split("correct ")[1]?.trim();
const { content } = tokens[index + 1];
const state = encodeToBase64(
JSON.stringify({
...(parserOptions && { options: { parserOptions: JSON.parse(parserOptions) } }),
text: content
})
);
harish-sethuraman marked this conversation as resolved.
Show resolved Hide resolved
const prefix = process.env.CONTEXT && process.env.CONTEXT !== "deploy-preview"
harish-sethuraman marked this conversation as resolved.
Show resolved Hide resolved
? ""
: "https://eslint.org";

return `
<div class="${name}">
<a class="c-btn c-btn--secondary c-btn--playground" href="${prefix}/play#${state}" target="_blank">
Open in Playground ↗️
</a>
`.trim();
}
}
];
}

const markdownIt = require("markdown-it");
const md = markdownIt({ html: true, linkify: true, typographer: true, highlight: (str, lang) => highlighter(md, str, lang) })
.use(markdownItAnchor, {
slugify: s => slug(s)
})
.use(markdownItContainer, "img-container", {})
.use(markdownItContainer, "correct", {})
.use(markdownItContainer, "incorrect", {})
.use(markdownItContainer, ...withPlaygroundRender("correct"))
.use(markdownItContainer, ...withPlaygroundRender("incorrect"))
.use(markdownItContainer, "warning", {
render(tokens, idx) {
return generateAlertMarkup("warning", tokens, idx);
Expand Down
18 changes: 0 additions & 18 deletions docs/src/assets/js/main.js
View file
Open in desktop
Expand Up @@ -192,24 +192,6 @@
}
})();

// add "Open in Playground" button to code blocks
// (function() {
// let blocks = document.querySelectorAll('pre[class*="language-"]');
// if (blocks) {
// blocks.forEach(function(block) {
// let button = document.createElement("a");
// button.classList.add('c-btn--playground');
// button.classList.add('c-btn');
// button.classList.add('c-btn--secondary');
// button.setAttribute("href", "#");
// button.innerText = "Open in Playground";
// block.appendChild(button);
// });
// }
// })();



// add utilities
var util = {
keyCodes: {
Expand Down
9 changes: 5 additions & 4 deletions docs/src/assets/scss/docs.scss
View file
Open in desktop
Expand Up @@ -142,13 +142,14 @@ pre[class*="language-"] {
.c-btn.c-btn--playground {
position: absolute;
font-size: var(--step--1);
bottom: 0.5rem;
right: 0.5rem;
bottom: -1rem;
right: 1rem;
offset-block-end: 0.5rem;
offset-inline-end: 0.5rem;
z-index: 1;

@media all and (max-width: 768px) {
display: none;
@media all and (min-width: 768px) {
bottom: 1rem;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This would be an accessibility issue. Users on less-wide screens, such as mobile phones and zoomed-in desktop displays, still would want to be able to access the Open in Playground button.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this, I'll have to ask @nzakas what the original intention was.

The Playground is quite good on small screens, but this button is very likely to cover the code:

Screenshot_2023-07-08-16-45-43-843_com android chrome

Though that can happen on wider screens too, depending on the text. Perhaps the button should be below the code block?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like the idea of moving the button below in mobile screens, yeah. Will wait to implement until directed to 👍

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, the intent was that smaller screens (mobile) would probably not want to use the playground as it's a bit clunky without a keyboard.

However, I'm not opposed to moving the button underneath the code block. That seems like a nice compromise to keep the functionality available without disrupting the UI too much.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Super, that's two votes in favor - I had a 💡 moment and moved the button to be half overlapping, half under the code block. That way it takes up much less vertical space and doesn't overlap lines of code.

}

Expand Down
93 changes: 46 additions & 47 deletions docs/src/rules/no-implicit-globals.md
View file
Open in desktop
Expand Up @@ -2,26 +2,25 @@
title: no-implicit-globals
rule_type: suggestion
related_rules:
- no-undef
- no-global-assign
- no-unused-vars
- no-undef
- no-global-assign
- no-unused-vars
further_reading:
- https://benalman.com/news/2010/11/immediately-invoked-function-expression/
- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Undeclared_var
- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let#Temporal_dead_zone
- https://benalman.com/news/2010/11/immediately-invoked-function-expression/
- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Errors/Undeclared_var
- https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/let#Temporal_dead_zone
---


It is the best practice to avoid 'polluting' the global scope with variables that are intended to be local to the script.

Global variables created from a script can produce name collisions with global variables created from another script, which will
usually lead to runtime errors or unexpected behavior.

This rule disallows the following:

* Declarations that create one or more variables in the global scope.
* Global variable leaks.
* Redeclarations of read-only global variables and assignments to read-only global variables.
- Declarations that create one or more variables in the global scope.
- Global variable leaks.
- Redeclarations of read-only global variables and assignments to read-only global variables.

There is an explicit way to create a global variable when needed, by assigning to a property of the global object.

Expand All @@ -32,7 +31,7 @@ By default, this rule does not check `const`, `let` and `class` declarations.

This rule has an object option with one option:

* Set `"lexicalBindings"` to `true` if you want this rule to check `const`, `let` and `class` declarations as well.
- Set `"lexicalBindings"` to `true` if you want this rule to check `const`, `let` and `class` declarations as well.

## Rule Details

Expand All @@ -44,7 +43,7 @@ This rule disallows `var` and `function` declarations at the top-level script sc

Examples of **incorrect** code for this rule:

::: incorrect
::: incorrect { "sourceType": "script" }

```js
/*eslint no-implicit-globals: "error"*/
Expand All @@ -58,28 +57,28 @@ function bar() {}

Examples of **correct** code for this rule:

::: correct
::: correct { "sourceType": "script" }

```js
/*eslint no-implicit-globals: "error"*/

// explicitly set on window
window.foo = 1;
window.bar = function() {};
window.bar = function () {};

// intended to be scope to this file
(function() {
var foo = 1;
(function () {
var foo = 1;

function bar() {}
function bar() {}
})();
```

:::

Examples of **correct** code for this rule with `"parserOptions": { "sourceType": "module" }` in the ESLint configuration:

::: correct
::: correct { "sourceType": "script" }

```js
/*eslint no-implicit-globals: "error"*/
Expand All @@ -100,7 +99,7 @@ This does not apply to ES modules since the module code is implicitly in `strict

Examples of **incorrect** code for this rule:

::: incorrect
::: incorrect { "sourceType": "script" }

```js
/*eslint no-implicit-globals: "error"*/
Expand All @@ -122,12 +121,12 @@ A read-only global variable can be a built-in ES global (e.g. `Array`), an envir
(e.g. `window` in the browser environment), or a global variable defined as `readonly` in the configuration file
or in a `/*global */` comment.

* [Specifying Environments](../use/configure#specifying-environments)
* [Specifying Globals](../use/configure#specifying-globals)
- [Specifying Environments](../use/configure#specifying-environments)
- [Specifying Globals](../use/configure#specifying-globals)

Examples of **incorrect** code for this rule:

::: incorrect
::: incorrect { "sourceType": "script" }

```js
/*eslint no-implicit-globals: "error"*/
Expand All @@ -149,13 +148,13 @@ Lexical declarations `const` and `let`, as well as `class` declarations, create
However, when declared in the top-level of a browser script these variables are not 'script-scoped'.
They are actually created in the global scope and could produce name collisions with
`var`, `const` and `let` variables and `function` and `class` declarations from other scripts.
This does not apply to ES and CommonJS modules.
This does not apply to ES and CommonJS modules.

If the variable is intended to be local to the script, wrap the code with a block or with an immediately-invoked function expression (IIFE).

Examples of **correct** code for this rule with `"lexicalBindings"` option set to `false` (default):

::: correct
::: correct { "sourceType": "script" }

```js
/*eslint no-implicit-globals: ["error", {"lexicalBindings": false}]*/
Expand All @@ -171,7 +170,7 @@ class Bar {}

Examples of **incorrect** code for this rule with `"lexicalBindings"` option set to `true`:

::: incorrect
::: incorrect { "sourceType": "script" }

```js
/*eslint no-implicit-globals: ["error", {"lexicalBindings": true}]*/
Expand All @@ -187,7 +186,7 @@ class Bar {}

Examples of **correct** code for this rule with `"lexicalBindings"` option set to `true`:

::: correct
::: correct { "sourceType": "script" }

```js
/*eslint no-implicit-globals: ["error", {"lexicalBindings": true}]*/
Expand All @@ -198,59 +197,59 @@ Examples of **correct** code for this rule with `"lexicalBindings"` option set t
class Bar {}
}

(function() {
(function () {
const foo = 1;
let baz;
class Bar {}
}());
})();
```

:::

If you intend to create a global `const` or `let` variable or a global `class` declaration, to be used from other scripts,
be aware that there are certain differences when compared to the traditional methods, which are `var` declarations and assigning to a property of the global `window` object:

* Lexically declared variables cannot be conditionally created. A script cannot check for the existence of
a variable and then create a new one. `var` variables are also always created, but redeclarations do not
cause runtime exceptions.
* Lexically declared variables do not create properties on the global object, which is what a consuming script might expect.
* Lexically declared variables are shadowing properties of the global object, which might produce errors if a
consuming script is using both the variable and the property.
* Lexically declared variables can produce a permanent Temporal Dead Zone (TDZ) if the initialization throws an exception.
Even the `typeof` check is not safe from TDZ reference exceptions.
- Lexically declared variables cannot be conditionally created. A script cannot check for the existence of
a variable and then create a new one. `var` variables are also always created, but redeclarations do not
cause runtime exceptions.
- Lexically declared variables do not create properties on the global object, which is what a consuming script might expect.
- Lexically declared variables are shadowing properties of the global object, which might produce errors if a
consuming script is using both the variable and the property.
- Lexically declared variables can produce a permanent Temporal Dead Zone (TDZ) if the initialization throws an exception.
Even the `typeof` check is not safe from TDZ reference exceptions.

Examples of **incorrect** code for this rule with `"lexicalBindings"` option set to `true`:

::: incorrect
::: incorrect { "sourceType": "script" }

```js
/*eslint no-implicit-globals: ["error", {"lexicalBindings": true}]*/

const MyGlobalFunction = (function() {
const MyGlobalFunction = (function () {
const a = 1;
let b = 2;
return function() {
return function () {
return a + b;
}
}());
};
})();
```

:::

Examples of **correct** code for this rule with `"lexicalBindings"` option set to `true`:

::: correct
::: correct { "sourceType": "script" }

```js
/*eslint no-implicit-globals: ["error", {"lexicalBindings": true}]*/

window.MyGlobalFunction = (function() {
window.MyGlobalFunction = (function () {
const a = 1;
let b = 2;
return function() {
return function () {
return a + b;
}
}());
};
})();
harish-sethuraman marked this conversation as resolved.
Show resolved Hide resolved
```

:::
Expand All @@ -261,7 +260,7 @@ You can use `/* exported variableName */` block comments in the same way as in [

Examples of **correct** code for `/* exported variableName */` operation:

::: correct
::: correct { "sourceType": "script" }

```js
/* exported global_var */
Expand Down