Repository navigation
Conversation
Keep line endings, final newlines, trailing whitespace and indentation consistent across editors for files clang-format does not cover.
Strip trailing whitespace and convert two CRLF files to LF. Markdown is left untouched since trailing spaces can be significant there. Whitespace-only change: git diff -w shows nothing.
Replace the legacy .eslintrc.yaml/.eslintignore, which ESLint 9+ no longer reads and which ignored every package, with eslint.config.mjs. - eslint 10, @eslint/js and typescript-eslint recommended presets, eslint-plugin-wc best-practice, eslint-plugin-mocha recommended for tests - drop the deprecated eslint-config-google; clang-format owns formatting - no-explicit-any is a warning for now - mocha/no-mocha-arrows and no-unused-expressions are off in tests to match the arrow-function style and chai assertions - npm run lint now lints every package, including JS/MJS files
Output of `eslint . --fix` (curly, prefer-const, no-var, explicit-member-accessibility, no-wrapper-object-types, prefer-as-const, no-extra-boolean-cast, ...). Changed lines were reformatted with git-clang-format. Vendored third_party code is now ignored.
- drop unnecessary backslashes in regular expressions and strings - use strict equality; comparisons against undefined become == null to keep matching both null and undefined - dropdown compares Number(selected), since paper-listbox's selected may be a string
- super.x && super.x() becomes super.x?.() - use optional catch bindings where the error is unused - replace comma operators with statements in camera_settings - drop type-only $CombinedState imports; declarations still emit - use the Category interface instead of a placeholder constant - comment out imports only used by commented-out test-runner config
- controls: keep the elapsed interaction-prompt time across visibility changes; it was reset on every call - validation: compare severityTitle instead of assigning it, so the glTF URL is only revoked after a conversion - exportScene: only wrap exporter.parse in the Promise, so errors from the awaited setup reject instead of leaving it pending; drop async from Promise executors that never await - custom-select example: set up shadow-DOM listeners once in the constructor and remove the document listener on disconnect - compare-results: read JSON with fs, as require() is unavailable in the ESM output - scene-graph-spec: the variant-revert tests returned from inside the search loop and never asserted; use break. The 'Yellow Red' case fails once it runs, so it is skipped with a TODO
- wrap lexical declarations in case blocks
- Object.prototype.hasOwnProperty.call instead of obj.hasOwnProperty
- replace Function and {} types with specific types
- assert non-null instead of mixing ?. with ! in tests
- attach caught errors as cause (adds es2022.error lib to
render-fidelity-tools)
- drop unneeded @ts-ignore comments, this aliases, dead initial
assignments and a full-width space
- comment empty catch blocks
- turn off no-unexpected-multiline, which conflicts with clang-format's
layout of calls and tagged templates
Lint runs right after npm ci so it fails fast. Warnings (no-explicit-any) do not fail the build.
- Lit update/updated overrides take Map<string|number|symbol, unknown>, matching existing usage; one caller now casts the old value - throttle/debounce are generic over the wrapped function's arguments - type render-fidelity configs as ImageComparisonConfig and let puppeteer/child_process handler types be inferred - catch bindings are unknown and cast to Error where .message is read - allow `...args: any[]` (ignoreRestArgs), which TypeScript mixins require no-explicit-any warnings: 269 -> 217.
User-Agent Client Hints are missing from TypeScript's DOM lib, so declare the one method we use and move the XR form-factor check into a helper. Behavior is unchanged.
lib/ is published and its .d.ts files are public. Restore the original throttle/debounce (any) and getValueOfEnum (T extends Object) signatures so callers that deep-import them are not broken by stricter types.
The zizmor check fails on pull requests that change a workflow file containing actions referenced by tag. Pin every action to the commit its current tag points to, with the version in a comment, and give the read-only workflows an explicit `contents: read` permission.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Reference Issue
None. Linting has effectively been off: ESLint 9 no longer reads
.eslintrc.yaml/.eslintignore, and the ignore file excluded every package anyway (it was set up in #992 to lint only the since-removed 3DOM package). The lint step in CI was commented out.Summary
Upgrades to ESLint 10 with a flat config, lints every package, fixes all errors, and turns lint back on in CI. clang-format stays the formatter; ESLint only checks correctness.
Tooling
eslint.config.mjsreplaces.eslintrc.yamland.eslintignore. It uses the recommended presets:@eslint/js,typescript-eslint,eslint-plugin-wc(best-practice) andeslint-plugin-mochafor tests. These presets contain no formatting rules, so they don't conflict with clang-format.eslint-config-google.npm run lintnow covers all packages, including JS/MJS build scripts and examples. Ignored:.gitignored output,*.d.ts,shared-assetsand vendoredthird_party/code.no-explicit-anyis a warning for now (216 left), withignoreRestArgsbecause TypeScript mixins need...args: any[].mocha/no-mocha-arrowsis off, to match the test style.no-unused-expressionsis off in tests, for chai assertions likeexpect(x).to.be.true.no-unexpected-multilineis off: clang-format itself puts call arguments and tagged templates on the next line..editorconfig(2-space indent, LF, final newline, trim trailing whitespace except in Markdown) and a whitespace-only cleanup commit (git diff -wis empty). These are the first two commits and can be split into their own PR if preferred.unit-tests.ymlright afternpm ci. Warnings don't fail the build.Fixes for lint errors
Mostly mechanical: braces,
const, strict equality, removed useless escapes, block-scopedcasebodies,Object.prototype.hasOwnProperty.call, specific types instead ofFunction/{}, and removed unused code. Changed lines were reformatted withgit clang-formatonly.Lint also found real bugs, fixed in their own commit:
if (this.severityTitle = 'Converted')assigned instead of compared, so the model URL was revoked on every model change.exportScene(): if the setup before the export failed, the returned promise never settled. Onlyexporter.parseis now wrapped in the Promise.custom-select.jsexample: listeners were added again on every reconnect, and thedocumentlistener was never removed.render-fidelity-toolscompare-results.ts: usedrequire()in ESM output, so it couldn't run.scene-graph-spec.ts: two variant testsreturned from inside their search loop and never asserted anything. Withbreak, theInversetest passes. TheYellow Redtest fails (yellowinstead ofred), so it istest.skipped with a TODO; it needs a separate look.No breaking changes
I built
masterand this branch (production build) and compared the unminifieddistbundles ofmodel-viewerandmodel-viewer-effects:?., since TypeScript compiles it away. The==→===changes only compare values of the same type, including where public properties are involved (glitchstrengthis clamped to a number first). GLSL template strings only lost trailing whitespace.model-viewer.min.jsis 23 bytes smaller.lib/*.d.tsis published):updated(changedProperties)is now typedMap<…, unknown>instead ofany. A consumer file overridingupdatedwithMap<string, any>, Lit'sPropertyValuesor the new type compiles against both versions. Helper signatures that would have become stricter (throttle,debounce,getValueOfEnum) were kept as they were.Testing
npm run lint: 0 errors, 216 warnings (no-explicit-any).tsc --noEmitpasses for all five packages.masteris the newly asserting variant tests described above. The full suite was not run locally.Notes
clang-formatpackage ships an x86-only macOS binary, so formatting on Apple Silicon used Homebrew clang-format 23, and only on changed lines.huskypre-commit hook inpackage.jsonstill uses the husky v4 format, which husky 9 ignores. Not changed here.