Repository navigation
Task/review 0.12 clean - #15
Merged
Merged
Conversation
The review bot reported two ERROR classes (the rest are warnings):
1. 'Disabling obsidianmd/prefer-create-el is not allowed' (harness) — the
harness implements Obsidian's DOM helpers, so the rule flagged its own
implementation. The shim now builds elements through a narrowly typed
handle (the same escape hatch the export code uses for its detached
DOMParser document), so no suppression is needed at all.
2. 'Disabling @typescript-eslint/no-explicit-any is not allowed' (22 files
in texto/core) — the point-wise suppressions of the covariance defaults
are gone. The types are reworked instead:
- AnyRecord is Record<string, unknown>; AnyFn takes never[] params
- the extension generics are constrained to 'object' and default to
'object' — the widest honest type, so Node<CustomOptions> stays
assignable to Node (covariance) without 'any'
- the config handlers are METHODS, so their 'this' is compared
bivariantly and a narrow config stays assignable to the loose one
- the command/node-view helpers read their own options through a cast
Also in this pass:
- the build tag is a source hash instead of Date.now(): the review rebuilds
the plugin and compares it with the released asset, and a timestamp made
every rebuild differ ('Build output does not match the released main.js')
- README: the plugin is ~6 MB, so Obsidian Sync (Standard) does not sync
the plugin folder — install or update it per device
- CHANGELOG: the size note and the settings-value fix
CSS stays as is: 21 browser-feature warnings are documented exceptions
(the review ignores stylelint-disable comments, and the alternatives cost
the underline styling, the mermaid editor layout or theme safety).
Gates: lint:src 0, lint:css 0, typecheck, 643 tests, 48/48 e2e, build —
main.js 6,091 kB, deterministic across builds
Live: Obsidian 1.14.4 (settings show saved values, mermaid renders) and
Android (mermaid renders, table cascade holds)
The shipped-code gate (lint:src) was already clean; these were the repo-wide warnings from `npm run lint` — mostly in the test files: - typed the shared obsidian mock (src/__mocks__/obsidian.ts): an untyped vi.fn() returns `any`, and every test reading mock.calls inherited it - typed the per-test mocks (media/attachment/image): the dispatch transaction, the choice dialog callback, the image update attrs - narrowed the JSON.parse results in the storage tests to the fields the test reads (no more `any` leaking into the assertions) - the DOM helpers come from the Obsidian runtime: the tests use createDiv() like the plugin code does (the vitest setup installs them) - noteStorage reads the write-log flag through window.localStorage — the test now does the same instead of the bare global (the rule wants the plugin to go through the app, and the bare global is banned) - the um container test compares the serialized bytes element-wise instead of through Node's Buffer (Buffer is not defined in the browser globals the lint config uses) - window.setTimeout in the notepad test, sentence case in a fixture title, two unused variables removed Gates: npm run lint 0 warnings, lint:src 0, typecheck, 643 tests, 48/48 e2e, build — pass
[Medium impact] The changes reach shared editor extension contracts and the plugin build, with runtime effects bounded to the editor and its distribution. The PR appears safe to merge, with non-blocking improvements needed for extension types and reproducible builds. FindingsSummaryThis PR removes explicit
Reviews (1) · Last reviewed commit: "chore(lint): clear the remaining eslint ..." · Reviewed by Greptile |
…e build hash Greptile P2 (NodeConfig.ts): the config handlers' `this` context had been widened to `object` to keep the concrete extension assignable to the framework's union — but that dropped the type of the extension's own options/storage and forced casts in the image, attachment and task code. Instead of widening the context, the handlers' `this` is now bivariant (the method-extract trick, BivariantFn) — the union members that hold a function keep their precise Options/Storage, and a narrow config still assigns to the wide one. AnyExtension became a structural type for the same reason. The casts the widening had forced are gone: the type checks are back. Greptile P2 (vite.config.mts): the source hash mixed the platform path separators, so the same sources hashed differently on Windows and the release build (Linux) and the review's rebuild carried different build tags. The relative path is normalized to forward slashes now. Gates: npm run lint 0, lint:src 0, typecheck, 643 tests, 48/48 e2e, build — main.js 6,091 kB, deterministic across builds Live: Obsidian 1.14.4 (settings show the saved value, Mermaid renders)
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.
No description provided.