Skip to content

fix: resolve Obsidian plugin-review findings (1.2.1) - #34

Merged
jphan32 merged 1 commit into
devfrom
fix/plugin-review-findings
Jul 28, 2026
Merged

fix: resolve Obsidian plugin-review findings (1.2.1)#34
jphan32 merged 1 commit into
devfrom
fix/plugin-review-findings

Conversation

@jphan32

@jphan32 jphan32 commented Jul 28, 2026

Copy link
Copy Markdown
Member

Two warnings from Obsidian's plugin review, and the reason our own gate did not catch either.

The findings

Source Finding
Source code Uses document.createElement instead of Obsidian's createEl helpersobsidianmd/prefer-create-el, src/main.ts:96
CSS lint Unexpected browser feature "text-decoration" is only partially supportedstyles.css:449

Both are real. el() created elements with activeDocument.createElement, and the inert footnote marker's cue used the multi-value text-decoration: underline dotted shorthand.

The fixes

  • el() now calls the free createEl(tag, { cls }). Deliberately not Node.createEl — that one appends to the node it is called on, and el() has to return a detached element.
  • The dotted underline is written as text-decoration-line + text-decoration-style. Both longhands predate the shorthand's multi-value form by a wide margin.

Why CI was green while both stood

This is the part worth reviewing.

  1. eslint-plugin-obsidianmd was pinned at 0.3.0, which does not carry prefer-create-el. The reviewer runs a newer ruleset, so our gate was checking a strictly smaller set of rules than the thing it exists to pre-empt. Bumped to 0.4.1, which reproduces the finding exactly:

    96:16  warning  Use 'activeWindow.createEl(tag)' instead of
                    'activeDocument.createElement(tag)'   obsidianmd/prefer-create-el
    
  2. That rule reports at warning severity, and eslint exits 0 on warnings. Even on 0.4.1 the gate would have passed. npm run lint now runs with --max-warnings=0.

Verified the hardened gate bites — reverting el() to activeDocument.createElement fails lint rather than passing with a warning.

Test harness

The e2e bootstrap gained a createEl / Node.prototype.createEl stub written to the real signature (second argument is a class string or a DomElementInfo), not shaped to whatever the bundle happens to call. A stub fitted to the implementation is exactly how a green suite hid a completely broken feature earlier in this release.

Verification

npm run lint            PASS   (--max-warnings=0, plugin 0.4.1)
npm run build           PASS
node --check main.js    PASS
node scripts/validate.mjs   PASS   (versions.json has 1.2.1 → 1.6.6)
npm run test:e2e        PASS   119 checks

Version 1.2.1 — compliance only, no behaviour change.

🤖 Generated with Claude Code

…let them through

Two findings from the plugin review, plus the reason our own gate missed both.

- `el()` builds elements with Obsidian's `createEl` helper instead of
  `document.createElement` (`obsidianmd/prefer-create-el`). The free function,
  not `Node.createEl` — the latter appends to the node it is called on, and
  `el()` must return something detached.
- The inert footnote marker's dotted underline uses the `text-decoration-line`
  / `text-decoration-style` longhands. Obsidian's CSS check flags the
  multi-value `text-decoration` shorthand as only partially supported at our
  `minAppVersion`.

Why CI was green while both stood:

- `eslint-plugin-obsidianmd` was pinned at 0.3.0, which does not carry the
  `prefer-create-el` rule the reviewer ran. Bumped to 0.4.1, which reproduces
  the finding exactly.
- That rule reports at *warning* severity, and `eslint` exits 0 on warnings, so
  even on 0.4.1 the gate would have passed. `npm run lint` now runs with
  `--max-warnings=0`.

Verified the hardened gate actually bites: reverting `el()` to
`activeDocument.createElement` fails `npm run lint`.

The e2e bootstrap gained a `createEl`/`Node.prototype.createEl` stub written to
the real signature — the second argument is a class string or a DomElementInfo
— rather than shaped to whatever the bundle happens to call. A stub fitted to
the implementation is how a green suite hid a broken feature earlier in this
release.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jphan32
jphan32 merged commit 4c360f9 into dev Jul 28, 2026
1 check passed
@jphan32
jphan32 deleted the fix/plugin-review-findings branch July 28, 2026 12:23
@jphan32 jphan32 mentioned this pull request Jul 28, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant