Change Request: global variable only in scope function

[regseb]

ESLint version

v8.16.0

What problem do you want to solve?

I use Playwright to control a browser from a Node script. With Playwright, you can write JavaScript that will be executed in the browser (with page.evaluate(pageFunction[, arg])).

import { chromium } from "playwright";

const browser = await chromium.launch();
const page = await browser.newPage();

// Execute JavaScript in browser.
await page.evaluate(() => {
    window.foo = "bar";
});

await browser.close();

ESLint reports that window isn't defined (no-undef). I add /* global window */:

await page.evaluate(() => {
    /* global window */
    window.foo = "bar";
});

But if window is used in the rest of the Node script, ESLint will not report the problem.

What do you think is the correct solution?

I propose that globals variables be added only in the block scope of comment /* global window */ (like a variable declared with let).

import { chromium } from "playwright";

const browser = await chromium.launch();
const page = await browser.newPage();
// Execute code in browser.
await page.evaluate(() => {
    /* global window */
    window.foo = "bar"; // ESLint no report no-undef
});

window.baz = "qux"; // ESLint report no-undef

await browser.close();

Participation

• I am willing to submit a pull request for this change.

Additional comments

No response

[mdjermanovic]

I think this problem is much more complex than just handling globals. Are variables that appear to be in upper scopes available in the function?

const x = 1;
await page.evaluate(() => {
    console.log(x); // should ESLint report `no-undef` on `x` here?
});

[regseb]

The variables in the upper scope aren't available in the function. I think Page.evaluate() does a toString() on the function and sends the string to the browser. For this problem, I don't find a solution to solve it.

[mdjermanovic]

The variables in the upper scope aren't available in the function. I think Page.evaluate() does a toString() on the function and sends the string to the browser. For this problem, I don't find a solution to solve it.

It seems that the right solution would be to lint those functions separately, which might be a good case for a custom processor that would split the file into code blocks, for which different override configurations could then be defined. I’m curious what other team members think.

[nzakas]

Having spent a bunch of time with Puppeteer, I can confirm that this is more complex than it appears because some functions are not closures depending on which method they are passed into.

I also think any change to how the global comment works would be a massive breaking change that could cause a lot of issues for existing code bases.

A possible option would be to add another directive comment that flags a function as a non-closure and then apply global just to that function rather than the whole file. A bit hacky but could work.

[nzakas]

@eslint/eslint-tsc what do we want to do here?

[btmills]

I agree that a custom processor would be the way to go. It would have specific knowledge of the library's semantics and know which function bodies to include in the main file lint and which to extract to a block. This will depend on #14745 to do it ergonomically, but there are examples in that thread of it being possible to do currently.

[github-actions]

Oops! It looks like we lost track of this issue. What do we want to do here? This issue will auto-close in 7 days without an update.