Skip to content

[WC-3504][WC-3264] update dependencies and add subscript and superscript - #2406

Open
gjulivan wants to merge 1 commit into
mainfrom
markdown/update
Open

[WC-3504][WC-3264] update dependencies and add subscript and superscript#2406
gjulivan wants to merge 1 commit into
mainfrom
markdown/update

Conversation

@gjulivan

Copy link
Copy Markdown
Collaborator

Pull request type


Description

@gjulivan
gjulivan requested a review from a team as a code owner August 31, 2026 22:49
@github-actions

This comment has been minimized.

@github-actions

Copy link
Copy Markdown
Contributor

AI Code Review

🚨 Blocked — high-severity issue (security, data loss, broken API) must be fixed


What was reviewed

File Change
packages/pluggableWidgets/markdown-web/src/Markdown.tsx Added markdown-it-sub and markdown-it-sup plugins
packages/pluggableWidgets/markdown-web/typings/markdown-it-sub.d.ts New ambient type declaration for markdown-it-sub
packages/pluggableWidgets/markdown-web/typings/markdown-it-sup.d.ts New ambient type declaration for markdown-it-sup
packages/pluggableWidgets/markdown-web/package.json Upgraded markdown-it ^14→^15, added sub/sup deps, bumped @types/markdown-it
packages/pluggableWidgets/markdown-web/CHANGELOG.md Added Unreleased entries for the dep update and new plugin support

Skipped (out of scope): pnpm-lock.yaml (lockfile) — see critical finding below


Findings

🚨 Critical — Unresolved merge conflict in pnpm-lock.yaml

File: pnpm-lock.yaml lines ~378–385 and ~1914–1921
Problem: The lockfile contains raw <<<<<<< HEAD / ======= / >>>>>>> conflict markers. This means the file is broken — pnpm install will fail for any developer who checks out this branch, and CI installs will also fail. A corrupt lockfile blocks the entire repo from being built or tested.
Fix: Resolve the conflict by choosing the correct version of @mendix/pluggable-widgets-tools (likely 11.13.0 from HEAD/main), then re-run pnpm install to regenerate a clean lockfile, and commit the result.


🔶 Medium — No tests covering the new subscript/superscript rendering

File: packages/pluggableWidgets/markdown-web/src/__tests__/Markdown.spec.tsx
Problem: The existing test suite has no cases for the new ~subscript~ and ^superscript^ syntax that this PR enables. Because the plugins are registered on the module-level mdParser singleton, a test case would exercise real rendering. Without it, a future plugin misconfiguration (wrong call order, wrong import) would not be caught.
Fix: Add test cases, for example:

it("renders subscript", () => {
    defaultProps.stringAttribute = new EditableValueBuilder<string>()
        .withValue("H~2~O")
        .build();
    const { container } = render(<Markdown {...defaultProps} />);
    expect(container.querySelector("sub")).toBeInTheDocument();
});

it("renders superscript", () => {
    defaultProps.stringAttribute = new EditableValueBuilder<string>()
        .withValue("E=mc^2^")
        .build();
    const { container } = render(<Markdown {...defaultProps} />);
    expect(container.querySelector("sup")).toBeInTheDocument();
});

⚠️ Low — Snapshot will silently pass after plugin addition without regeneration

File: packages/pluggableWidgets/markdown-web/src/__tests__/__snapshots__/Markdown.spec.tsx.snap
Note: The existing snapshot test renders "Markdown viewer default value" which does not exercise the new plugins, so the snapshot has not changed. This is fine for now, but the snapshot test gives no signal about sub/sup rendering correctness — it only detects accidental structural regressions on plain-text input. The suggested test cases above (using specific DOM assertions) are preferable over relying on snapshots for new behaviour.


⚠️ Low — PR template sections are not filled in

File: PR description
Note: All PR type checkboxes remain commented out, and the Description section is empty. This PR adds new user-visible functionality (sub/superscript support) and bumps a major dependency version — both warrant a short description of what was changed and why, plus guidance on what to test.


Positives

  • Correct use of MarkdownIt.PluginSimple type in the ambient declarations — avoids any and keeps the type-safe .use() call chain.
  • CHANGELOG entries follow the Keep a Changelog format correctly and are placed under [Unreleased] (not version-bumped), matching repo convention.
  • The module-level parser initialisation with .use(sub).use(sup) is idiomatic for markdown-it and avoids re-registering plugins on every render.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant