chore(lint): enable no-undef for the native browser shims

[moodyjmz]

Follow-up to #67. Merge #67 first — see "Expected CI state" below.

Why

common/Native/ stubs window, document, navigator, console, setTimeout and performance for the embedded JS context that DoctRenderer/x2t runs. It is the one part of this tree that no test ever executes: tests/runAll.js and the unit-tests job drive QUnit in a real browser via node-qunit-puppeteer, where these stubs are never loaded and the real platform objects are used instead.

So an undeclared identifier in these files reaches production entirely unexercised. That is exactly how console.error came to reference an undeclared param and abort every converter operation that logged an error — silent save failure, ExitCode 86, data loss. Five weeks of dormancy, then production.

no-undef catches it. .eslintrc.yaml did not enable it, and nothing ran eslint at all — there is no eslint reference in build/package.json, no root package.json, and the code-style job only ran tests/code-style/check.py (license headers, LF endings, trailing newline).

Proof it's a real guard

Against main as it stands today:

$ eslint --format unix 'common/Native/*.js'
common/Native/native.js:340:78: 'param' is not defined. [Error/no-undef]

1 problem

With #67's one-word fix applied: clean, exit 0. One error, the right error, no noise.

Why it's scoped to common/Native

Repo-wide, no-undef reports 85,854 violations across 1,437 distinct identifiers, measured per directory (common 26,446 / word 27,779 / cell 15,658 / slide 7,574 / pdf 6,543 / visio 1,854). The top offenders are legitimate cross-module namespaces:

identifier	count
AscCommon	16,525
AscDFH	16,021
Asc	13,402
AscFormat	12,797
AscCommonExcel	4,712

Making that green would need a globals list larger than the rule is worth, so this PR does not attempt it. Widening the scope later is a separate conversation with a separate cost.

Why browser: false

Counter-intuitive but load-bearing. These files provide the browser globals rather than consuming them. With browser: true the existing no-redeclare rule fires on every stub — navigator, document, console, setTimeout, setInterval, clearTimeout, clearInterval, performance, Image — and buries the one real defect in eleven false ones. I hit exactly that while building this; the first draft reported 12 problems, of which 11 were noise.

With browser: false, the three globals the converter genuinely injects before any script runs are declared explicitly, each pointing at where core installs it:

global	installed by
window (the global object itself)	doctrenderer.cpp:579 — global_js->set("window", global_js)
native	doctrenderer.cpp:583 — global_js->set("native", oNativeCtrl)
CreateEmbedObject	js_internal/v8/v8_base.cpp:218 — InsertToGlobal(...)

Split into two per-file override blocks because native.js declares Asc/AscCommon/AscFormat/AscDFH itself, whereas native_graphics.js consumes AscCommon from it — editors.cpp (GetAllScript) concatenates them in that order. Declaring AscCommon globally for both would trip no-redeclare on native.js's own var AscCommon = {}.

jquery_native.js is ignored via .eslintignore, matching tests/code-style/check.py's own exclude_files.

Expected CI state

code-style will fail on this branch until #67 merges, with exactly:

common/Native/native.js:340:78: 'param' is not defined. [Error/no-undef]

That failure is the demonstration, not a defect in this PR. Merge #67, and this goes green with no changes.

Divergence cost

.eslintrc.yaml and .github/workflows/check-build.yml both exist upstream, so this is real fork divergence. It is confined to an overrides: block appended to the end of one file and one step added to one job — upstream touches neither often — and .eslintignore is a new file, so it cannot conflict. Judged worth it: the alternative is relying on nobody ever again writing an undeclared identifier in the only directory CI cannot reach.

[moodyjmz]

CI has now run, and the guard behaves exactly as intended. code-style fails with a single error and nothing incidental:

340:78  error  'param' is not defined  no-undef
✖ 1 problem (1 error, 0 warnings)
Process completed with exit code 1

DCO, build-test and unit-tests all pass. The code-style failure is main's existing defect being caught, not a problem with this PR — it clears once #67 merges. Steps 1–5 of the job (checkout, python, check.py, node setup) all passed, so eslint genuinely ran and rejected the code rather than the step falling over.

Two caveats in the lint native shims step that reviewers should weigh

Both are visible in the CI log but not stated anywhere in the PR, which is a gap on my part.

1. eslint 8.57.1 is end-of-life. The log says so on every run:

npm warn deprecated eslint@8.57.1: This version is no longer supported

I pinned 8.x because eslint 9 requires flat config (eslint.config.js), and .eslintrc.yaml is an upstream-owned file. Migrating it would convert a 60-line change into a config migration with a much larger divergence footprint and a guaranteed conflict on the next ONLYOFFICE sync. That trade still looks right to me, but the consequence is that this step is pinned to a dead-end major version and the warning will nag indefinitely.

2. npx --yes fetches eslint from the registry on every CI run. No lockfile, no integrity pin, no cache. That means:

• a supply-chain surface — the step resolves and executes a freshly-downloaded dependency tree each run, with only the version string pinned
• a build-reliability dependency on npm being reachable, in a job that previously needed nothing but Python
• roughly a second of install time per run, which is noise, but it is not zero

The cleaner alternative, and why I didn't just do it

A root package.json with eslint as a pinned devDependency, npm ci, and actions/setup-node's cache: npm. That gets a lockfile, integrity hashes, and a warm cache — strictly better on all three counts above.

I did not do it because sdkjs has no root package.json at all — only build/package.json. Introducing one to a mirrored fork is a structural change with consequences beyond this PR (it changes what tooling infers about the repo root, and it is a new file upstream may later add itself), and that is a call for whoever owns the repo's build surface rather than something to slip into a lint fix.

So: the scoped rule and the config are what I am actually asking to be reviewed here. If the lockfile is worth a root package.json, say so and I will do it as a follow-up rather than expanding this PR.

[moodyjmz]

Merge #65 before this PR

#65 (the Grunt → webpack build migration) touches two of the same files, and one of them is a guaranteed conflict. It's the larger, older PR and it's green — this one should rebase onto it, not the other way round.

1. .eslintignore — certain add/add conflict. Both PRs create the file, with disjoint content:

	contents
#65	deploy/, develop/, build/node_modules/, vendor/, common/zlib/, common/libfont/, pdf/src/engine/drawingfile_ie.js, pdf/build/, tests/
this PR	common/Native/jquery_native.js

Whichever lands second conflicts, and the reflexive "take theirs" resolution silently discards the other side's intent. The resolution must be the union of both lists — #65's entries are the useful baseline for any future repo-wide lint, and this PR's single entry is what the scoped common/Native rule needs in order to skip the vendored jQuery fork.

Worth noting the two are complementary rather than redundant: #65 adds an .eslintignore but never actually runs eslint anywhere — its code-style job is still only python tests/code-style/check.py. This PR is what wires eslint in. So today #65's ignore file is inert and this one is load-bearing; after both land, they're one list serving one invocation.

2. check-build.yml — overlapping but probably benign. #65 doesn't modify the code-style job at all (it changes the on: triggers and the other two jobs), while this PR appends two steps to code-style. Different regions, so git should cope. Flagging it because this PR's insertion point sits immediately before unit-tests:, which #65 does modify, so the context lines may brush. Lower confidence than the .eslintignore collision, which is certain.

Correction to this PR's description

The body states that check-build.yml has "a bare pull_request: trigger with no branch filter." That is true of main today and will be false once #65 merges — #65 restores the pull_request.branches filter and adds main to it (commit de2b1dd41e).

No functional impact on this PR: main is in the restored filter, so PRs targeting main still get CI. But the reasoning in the description goes stale, so don't read it as current after #65 lands.

Suggested order

1. fix: declare missing param in native console.error shim #67 — independent of all of this. It only touches common/Native/native.js, which build(sdkjs): fix webpack migration bundle bloat, CI, and coverage gaps #65 does not modify at all, and it's the one-line fix for the actual data-loss bug. No reason to hold it behind a 5,493-line build migration.
2. build(sdkjs): fix webpack migration bundle bloat, CI, and coverage gaps #65 — larger, older, green, and it should precede the upstream sync discussion in Update from master branch in upstream repository #21.
3. This PR — rebase onto build(sdkjs): fix webpack migration bundle bloat, CI, and coverage gaps #65, merging .eslintignore to the union above and dropping the stale trigger claim from the description.