Skip to content

Conversation

adrianschmidt
Copy link
Contributor

@adrianschmidt adrianschmidt commented Aug 22, 2025

Summary by CodeRabbit

  • New Features

    • None
  • Bug Fixes

    • Improved stability of the Date Picker by safely handling visibility checks.
  • Documentation

    • Enhanced API doc generation with pre-processing of tags and updated TSDoc configuration.
  • Tests

    • More resilient rendering tests for Color Picker and Split Button.
    • Added a global ResizeObserver mock to stabilize test runs.
  • Chores

    • Upgraded build tooling and Sass integration.
    • Configured Sass to resolve imports from node_modules.
  • Refactor

    • Consolidated text editor link types for clearer type sharing.

Review:

  • Commits are atomic
  • Commits have the correct type for the changes made
  • Commits with breaking changes are marked as such

Browsers tested:

(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)

Windows:

  • Chrome
  • Edge
  • Firefox

Linux:

  • Chrome
  • Firefox

macOS:

  • Chrome
  • Firefox
  • Safari

Mobile:

  • Chrome on Android
  • iOS

@adrianschmidt adrianschmidt requested a review from a team as a code owner August 22, 2025 13:26
Copy link

coderabbitai bot commented Aug 22, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

Configuration updates for API extraction, TSDoc, and Stencil/Sass; new preprocessing script to fix TSDoc tags and reroute API extractor to temp/types; several SCSS files add @use imports; tests switch to DOM-structure assertions and add ResizeObserver mock; minor TypeScript refactors for text editor link types; small runtime safety tweaks.

Changes

Cohort / File(s) Summary
Build & docs tooling
api-extractor.json, package.json, tsdoc.json, stencil.config.ts, stencil.config.docs.ts, stencil.config.dist.ts
API extractor points to temp/types; adds extractor message mapping; TSDoc base and supported tags adjusted; Stencil upgraded and Sass plugin now uses includePaths: ['node_modules']; test setup added to Stencil config; scripts run TSDoc-fix before extraction.
Preprocessing script
scripts/fix-tsdoc-tags.js
New Node script copies dist/types/**/*.d.ts to temp/types/** and converts selected JSDoc tags to TSDoc (@default@defaultValue, @private@internal); exports convertJSDocToTSDoc.
Testing updates
src/components/color-picker/color-picker.spec.tsx, src/components/split-button/split-button.spec.tsx, src/test-setup.ts
Spec tests now validate DOM structure vs. exact HTML; add global ResizeObserver mock via test setup.
Runtime code tweaks
src/components/date-picker/flatpickr-adapter/flatpickr-adapter.tsx, src/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsx, src/index.ts
Safe optional call for checkVisibility; update import path for linkMarkSpec; remove JSX popover augmentation from @stencil/core/internal.
Text editor link types refactor
src/components/text-editor/prosemirror-adapter/plugins/link/link-mark.ts, .../plugins/link/link-mark.types.ts, .../plugins/link/utils.ts
Extract LinkMarkAttrs to new link-mark.types.ts; update imports accordingly; no runtime changes.
SCSS @use imports
src/components/dock/partial-styles/shrink-expand-button.scss, src/components/file-viewer/partial-styles/ui-controls.scss, src/components/input-field/partial-styles/_readonly.scss, src/components/list/partial-styles/_has-grid-layout.scss, src/components/list/partial-styles/_static-actions.scss, src/components/progress-flow/progress-flow-item/partial-styles/_selected-indicator.scss, src/components/select/partial-styles/_readonly.scss, src/components/slider/partial-styles/_thumb.scss
Add @use imports for shared Sass functions, mixins, z-index, and shared_input-select-picker modules; no selector/property changes.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  actor Dev as Developer
  participant NPM as npm scripts
  participant Build as Stencil build
  participant Fix as fix-tsdoc-tags.js
  participant AE as API Extractor

  Dev->>NPM: run "api:update"
  NPM->>Build: stencil build
  Build-->>NPM: artifacts in dist/types
  NPM->>Fix: node scripts/fix-tsdoc-tags.js
  Fix->>Fix: copy dist/types → temp/types<br/>rewrite tags to TSDoc
  Fix-->>NPM: temp/types ready
  NPM->>AE: api-extractor (mainEntry: temp/types/index.d.ts)
  AE-->>Dev: API report updated

  Note over NPM,Fix: "api:verify" runs Fix before AE (no build)
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related issues

Possibly related PRs

Suggested labels

maintenance

Suggested reviewers

  • jgroth
  • LucyChyzhova
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch update-stencil

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

‼️ IMPORTANT
Auto-reply has been disabled for this repository in the CodeRabbit settings. The CodeRabbit bot will not respond to your replies unless it is explicitly tagged.

  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

CodeRabbit Commands (Invoked using PR/Issue comments)

Type @coderabbitai help to get the list of available commands.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

Status, Documentation and Community

  • Visit our Status Page to check the current availability of CodeRabbit.
  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 24

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (4)
src/components/text-editor/prosemirror-adapter/plugins/link/utils.ts (2)

31-44: Treat cross-origin (including different ports) as external by comparing origin, not just hostname

Using only hostname misclassifies URLs on the same host but different port as internal. Cross-origin also includes port differences.

Apply this diff:

-        return (
-            urlObj.protocol.startsWith('http') &&
-            urlObj.hostname !== window.location.hostname
-        );
+        const isHttp = urlObj.protocol === 'http:' || urlObj.protocol === 'https:';
+        if (!isHttp) {
+            return false;
+        }
+        // Cross-origin includes protocol, host, and port
+        return urlObj.origin !== window.location.origin;

8-29: Block unsafe URL schemes in getLinkAttributes to prevent XSS

Currently, a javascript:, data:, or similar unsafe URL will bypass isExternalUrl (and be treated as “internal”), leaving the raw href in the DOM and allowing script execution on click. We must guard against these schemes before any other checks.

• Location: src/components/text-editor/prosemirror-adapter/plugins/link/utils.ts
• Function: export function getLinkAttributes (lines 8–29)
• Action: Add an early guard using hasUnsafeScheme, neutralize unsafe links by returning an empty href and nullifying all other link attributes.

Suggested diff:

 export function getLinkAttributes(
     url: string,
     title: string | null
 ): LinkMarkAttrs {
+    // Prevent XSS by stripping out javascript:, data:, vbscript: links
+    if (hasUnsafeScheme(url)) {
+        return {
+            href: '',
+            title,
+            target: null,
+            rel: null,
+            referrerpolicy: null,
+        };
+    }
     if (isExternalUrl(url)) {
         return {
             href: url,
             title: title,
             target: '_blank',
             rel: 'noopener noreferrer',
             referrerpolicy: 'noreferrer',
         };
     }
@@
 }
 
 function isExternalUrl(url: string): boolean {
@@
 }
 
 function isProtocolRelativeUrl(url: string) {
     return url.startsWith('//');
 }
+
+/** Disallow known-dangerous schemes to prevent XSS via <a href> */
+function hasUnsafeScheme(url: string): boolean {
+    return /^\s*(javascript:|data:|vbscript:)/i.test(url);
+}

Add/adjust unit tests to verify:

  • javascript:alert(1) → returns href: '' and target, rel, referrerpolicy all null
  • https://example.com:8443/path → treated as external (target: '_blank')
  • mailto:[email protected], tel:+123456789, ftp://example.com → not external, preserved href
src/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsx (1)

585-593: Fix falsy check so position 0 doesn’t get ignored

this.lastClickedPos can be 0, which is falsy and will skip restoring the selection. Compare against null/undefined instead.

Apply this diff:

-                if (!this.transactionFired && this.lastClickedPos) {
+                if (!this.transactionFired && this.lastClickedPos != null) {
                     const { doc, tr } = this.view.state;
                     const resolvedPos = doc.resolve(this.lastClickedPos);
                     const selection = Selection.near(resolvedPos);
                     this.view.dispatch(tr.setSelection(selection));
                 }
src/components/text-editor/prosemirror-adapter/plugins/link/link-mark.ts (1)

27-36: Strengthen typing of toDOM’s mark parameter

Give mark a concrete type and import it from prosemirror-model. Keeps us honest if attr names change.

Apply this diff:

-import { MarkSpec, DOMOutputSpec } from 'prosemirror-model';
+import { MarkSpec, DOMOutputSpec, Mark } from 'prosemirror-model';
@@
-    toDOM: (mark): DOMOutputSpec => {
+    toDOM: (mark: Mark): DOMOutputSpec => {
         const target = mark.attrs.target || null;
 
         const securityAttrs = {
             rel: target === '_blank' ? 'noopener noreferrer' : null,
             referrerpolicy: target === '_blank' ? 'noreferrer' : null,
         };
 
         return ['a', { ...mark.attrs, ...securityAttrs }, 0];
     },
📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

💡 Knowledge Base configuration:

  • MCP integration is disabled by default for public repositories
  • Jira integration is disabled
  • Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between c2b86b8 and a5a1e86.

⛔ Files ignored due to path filters (3)
  • etc/lime-elements.api.md is excluded by !etc/lime-elements.api.md
  • package-lock.json is excluded by !**/package-lock.json
  • src/components/popover-surface/__snapshots__/popover-surface.spec.tsx.snap is excluded by !**/*.snap
📒 Files selected for processing (24)
  • api-extractor.json (3 hunks)
  • package.json (2 hunks)
  • scripts/fix-tsdoc-tags.js (1 hunks)
  • src/components/color-picker/color-picker.spec.tsx (1 hunks)
  • src/components/date-picker/flatpickr-adapter/flatpickr-adapter.tsx (1 hunks)
  • src/components/dock/partial-styles/shrink-expand-button.scss (1 hunks)
  • src/components/file-viewer/partial-styles/ui-controls.scss (1 hunks)
  • src/components/input-field/partial-styles/_readonly.scss (1 hunks)
  • src/components/list/partial-styles/_has-grid-layout.scss (1 hunks)
  • src/components/list/partial-styles/_static-actions.scss (1 hunks)
  • src/components/progress-flow/progress-flow-item/partial-styles/_selected-indicator.scss (1 hunks)
  • src/components/select/partial-styles/_readonly.scss (1 hunks)
  • src/components/slider/partial-styles/_thumb.scss (1 hunks)
  • src/components/split-button/split-button.spec.tsx (2 hunks)
  • src/components/text-editor/prosemirror-adapter/plugins/link/link-mark.ts (1 hunks)
  • src/components/text-editor/prosemirror-adapter/plugins/link/link-mark.types.ts (1 hunks)
  • src/components/text-editor/prosemirror-adapter/plugins/link/utils.ts (1 hunks)
  • src/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsx (1 hunks)
  • src/index.ts (0 hunks)
  • src/test-setup.ts (1 hunks)
  • stencil.config.dist.ts (1 hunks)
  • stencil.config.docs.ts (1 hunks)
  • stencil.config.ts (2 hunks)
  • tsdoc.json (2 hunks)
💤 Files with no reviewable changes (1)
  • src/index.ts
🧰 Additional context used
📓 Path-based instructions (5)
**/*.tsx

📄 CodeRabbit inference engine (.cursor/rules/wrap-multiple-jsx-elements-in-host-not-in-array.mdc)

When returning multiple JSX elements from the render method, never wrap them in an array literal. Instead, always wrap them in the special <Host> element. When there is already a single top-level element, it does not have to be wrapped in <Host>.

Files:

  • src/components/color-picker/color-picker.spec.tsx
  • src/components/date-picker/flatpickr-adapter/flatpickr-adapter.tsx
  • src/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsx
  • src/components/split-button/split-button.spec.tsx

⚙️ CodeRabbit configuration file

**/*.tsx: Our .tsx files are typically using StencilJS, not React. When a developer wants to return multiple top-level JSX elements from the render method, they will sometimes wrap them in an array literal. In these cases, rather than recommending they add key properties to the elements, recommend removing the hardcoded array literal. Recommend wrapping the elements in StencilJS's special <Host> element.

Files:

  • src/components/color-picker/color-picker.spec.tsx
  • src/components/date-picker/flatpickr-adapter/flatpickr-adapter.tsx
  • src/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsx
  • src/components/split-button/split-button.spec.tsx
**/*.{ts,tsx}

⚙️ CodeRabbit configuration file

**/*.{ts,tsx}: Imports from other files in the same module (lime-elements) must use relative paths. Using absolute paths for imports will cause the production build to fail.

Files:

  • src/components/color-picker/color-picker.spec.tsx
  • src/components/text-editor/prosemirror-adapter/plugins/link/link-mark.types.ts
  • src/test-setup.ts
  • src/components/date-picker/flatpickr-adapter/flatpickr-adapter.tsx
  • src/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsx
  • src/components/text-editor/prosemirror-adapter/plugins/link/utils.ts
  • src/components/split-button/split-button.spec.tsx
  • stencil.config.dist.ts
  • stencil.config.docs.ts
  • stencil.config.ts
  • src/components/text-editor/prosemirror-adapter/plugins/link/link-mark.ts
**/*.{tsx,scss}

⚙️ CodeRabbit configuration file

**/*.{tsx,scss}: Almost all our components use shadow-DOM. Therefore, we have no need of BEM-style class names in our CSS.

Files:

  • src/components/color-picker/color-picker.spec.tsx
  • src/components/list/partial-styles/_static-actions.scss
  • src/components/progress-flow/progress-flow-item/partial-styles/_selected-indicator.scss
  • src/components/date-picker/flatpickr-adapter/flatpickr-adapter.tsx
  • src/components/slider/partial-styles/_thumb.scss
  • src/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsx
  • src/components/file-viewer/partial-styles/ui-controls.scss
  • src/components/select/partial-styles/_readonly.scss
  • src/components/split-button/split-button.spec.tsx
  • src/components/list/partial-styles/_has-grid-layout.scss
  • src/components/dock/partial-styles/shrink-expand-button.scss
  • src/components/input-field/partial-styles/_readonly.scss
src/components/**/*.tsx

⚙️ CodeRabbit configuration file

src/components/**/*.tsx: When contributors add new props to existing components in the lime-elements repository, they should always add documentation examples that demonstrate the new prop's usage and explain how it works. This helps with user adoption, feature discoverability, and maintains comprehensive documentation.

Files:

  • src/components/color-picker/color-picker.spec.tsx
  • src/components/date-picker/flatpickr-adapter/flatpickr-adapter.tsx
  • src/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsx
  • src/components/split-button/split-button.spec.tsx
src/components/**/*.scss

📄 CodeRabbit inference engine (.cursor/rules/css-class-names-in-components-using-shadow-dom.mdc)

Do not use BEM-style class names in CSS for components, as components use shadow-DOM.

Files:

  • src/components/list/partial-styles/_static-actions.scss
  • src/components/progress-flow/progress-flow-item/partial-styles/_selected-indicator.scss
  • src/components/slider/partial-styles/_thumb.scss
  • src/components/file-viewer/partial-styles/ui-controls.scss
  • src/components/select/partial-styles/_readonly.scss
  • src/components/list/partial-styles/_has-grid-layout.scss
  • src/components/dock/partial-styles/shrink-expand-button.scss
  • src/components/input-field/partial-styles/_readonly.scss
🧠 Learnings (1)
📚 Learning: 2025-04-29T14:15:32.104Z
Learnt from: adrianschmidt
PR: Lundalogik/lime-elements#3529
File: src/components/ai-avatar/ai-avatar.scss:20-22
Timestamp: 2025-04-29T14:15:32.104Z
Learning: In components using Shadow DOM (like those in lime-elements), the universal selector `*` is already scoped to the component's shadow tree and won't cause unwanted side effects outside the component. There's no functional difference between `*` and `:host *` in this context.

Applied to files:

  • src/components/input-field/partial-styles/_readonly.scss
🪛 GitHub Check: Lint
scripts/fix-tsdoc-tags.js

[warning] 15-15:
Missing JSDoc comment


[failure] 13-13:
Do not use "__dirname"


[failure] 12-12:
Do not use "__dirname"


[failure] 10-10:
Do not use "require"


[failure] 10-10:
A require() style import is forbidden


[failure] 9-9:
Prefer node:path over path


[failure] 9-9:
Do not use "require"


[failure] 9-9:
A require() style import is forbidden


[failure] 8-8:
Prefer node:fs over fs


[failure] 8-8:
Do not use "require"


[failure] 8-8:
A require() style import is forbidden

⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
  • GitHub Check: Docs / Publish Docs
  • GitHub Check: Build
🔇 Additional comments (19)
src/components/text-editor/prosemirror-adapter/plugins/link/link-mark.types.ts (1)

6-6: Naming consistency verified

All occurrences of the attribute use the lowercase referrerpolicy spelling; no instances of referrerPolicy were found in the src/ tree. This aligns with the DOM attribute name and is consistently applied across:

  • markdown-parser.ts
  • link-markdown-plugin.ts
  • link-mark.types.ts
  • link/utils.ts (+ tests)
  • link-mark.ts

No further changes required.

src/components/text-editor/prosemirror-adapter/plugins/link/utils.ts (1)

1-1: Import path change LGTM

Import now points to the dedicated types module via a relative path as per repository guidelines. No runtime impact.

src/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsx (1)

38-38: Import path update LGTM and compliant with path guidelines

Now importing linkMarkSpec from the implementation file (link-mark) using a relative path. Matches repository rule for internal imports.

src/components/text-editor/prosemirror-adapter/plugins/link/link-mark.ts (1)

2-2: Type centralization LGTM

Importing LinkMarkAttrs from the new types module improves reuse and avoids circular deps. No behavioral change.

src/components/progress-flow/progress-flow-item/partial-styles/_selected-indicator.scss (1)

1-1: Z-index token is correctly available via legacy @import
I verified that $limel-progress-flow-step-content (and $limel-progress-flow-divider) are declared in src/components/progress-flow/progress-flow-item/progress-flow-item.scss (lines 4–6) before the @import './partial-styles/_selected-indicator' on line 183. Because _selected-indicator.scss is brought in with Sass’s legacy @import, it inherits those variable definitions without needing its own @use. All pxToRem calls across the component are properly namespaced as functions.pxToRem, so no additional changes are required.

src/components/list/partial-styles/_has-grid-layout.scss (1)

1-1: LGTM: Sass utilities brought in via @use

Importing the shared functions module is consistent with Sass’ module system and avoids relying on implicit globals. No issues spotted.

src/components/list/partial-styles/_static-actions.scss (1)

1-1: LGTM: explicit functions import

Bringing the functions module in explicitly is the right move with Stencil v4 + Sass modules.

stencil.config.docs.ts (2)

37-41: Sass plugin includePaths looks correct for Stencil v4

Switching to sass({ includePaths: ['node_modules'] }) aligns with the module system and makes node_modules resolution reliable in docs builds. Looks good.


37-41: Parity confirmed: main, docs, and dist configs share identical Sass plugin settings and no global style injections detected

No injectGlobalPaths or @use … as * usages were found anywhere in the repo, and all three Stencil configs (stencil.config.ts, stencil.config.docs.ts, and stencil.config.dist.ts) define the Sass plugin with the same includePaths: ['node_modules'] block. Style configuration is consistent across builds.

src/components/select/partial-styles/_readonly.scss (1)

1-1: No missing tokens in shared_input-select-picker

I’ve confirmed that src/style/internal/shared_input-select-picker.scss declares both $label-color (line 21) and $input-text-color (line 23), so your @use '../../../style/internal/shared_input-select-picker'; in _readonly.scss will indeed provide those variables as expected—no additional forwarding is required.

src/components/dock/partial-styles/shrink-expand-button.scss (1)

1-2: Sass modules imported with correct relative paths and namespacing — LGTM

Using @use with the mixins/functions namespaces matches the rest of the file and our shadow-DOM CSS guidelines. No issues.

src/components/slider/partial-styles/_thumb.scss (1)

1-2: Functions module import is correct and actively used — LGTM

functions.pxToRem is used throughout; the @use matches our Sass module pattern.

api-extractor.json (2)

429-433: Add ae-extra-release-tag to API report (non-blocking visibility) — LGTM

Routing ae-extra-release-tag as a warning and adding it to the API report aligns with the existing pattern for ae-missing-release-tag, improving PR diffs without failing local builds.


48-49: Preprocessing step validated

  • The api:update script (npm run build && node scripts/fix-tsdoc-tags.js && api-extractor run --local --verbose) and the api:verify script (node scripts/fix-tsdoc-tags.js && api-extractor run) both invoke node scripts/fix-tsdoc-tags.js before running API Extractor.
  • The scripts/fix-tsdoc-tags.js file is present in the repository.

No further changes needed for api-extractor.json—the preprocessing flow is correctly wired.

src/components/input-field/partial-styles/_readonly.scss (1)

1-2: SCSS modules import is correct and aligns with module-usage elsewhere.

The new @use imports properly scope z-index tokens and mixins.truncate-text(); the references further down resolve cleanly with namespacing. No BEM concerns introduced, selectors unchanged.

src/components/split-button/split-button.spec.tsx (1)

10-16: Good switch to DOM-structure assertions for the no-items case.

Assertions target host tag, conditional class, and key shadow children. This is more resilient than string snapshots.

stencil.config.ts (1)

57-57: Test setup registration is correct.

setupFilesAfterEnv pointing to src/test-setup.ts integrates the ResizeObserver mock into all tests. Path looks correct relative to repo root.

tsdoc.json (1)

3-3: LGTM: extending tsdoc-base aligns with API Extractor defaults

This simplifies custom maintenance and uses the recommended base config. No issues spotted.

scripts/fix-tsdoc-tags.js (1)

75-84: Hard failure when dist/types is missing is correct here, but ensure callers build first

process.exit(1) is appropriate to surface a missing build. With the proposed change to api:verify (build first), this path will be rare. No action needed if you adopt that script change.

package.json Outdated
Comment on lines 19 to 21
"api:update": "npm run build && node scripts/fix-tsdoc-tags.js && api-extractor run --local --verbose",
"api:verify": "node scripts/fix-tsdoc-tags.js && api-extractor run",
"build": "cross-env-shell NODE_ENV=prod SASS_PATH=node_modules \"stencil build --config stencil.config.dist.ts\"",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

api:verify will fail if dist/types isn’t present — ensure a build runs first

api:verify invokes the tag-fixer and API Extractor but never builds the types. On clean CI runs, dist/types won’t exist and the fixer exits with code 1, failing the job. Make api:verify build first.

Apply this diff:

-    "api:verify": "node scripts/fix-tsdoc-tags.js && api-extractor run",
+    "api:verify": "npm run build && node scripts/fix-tsdoc-tags.js && api-extractor run",
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"api:update": "npm run build && node scripts/fix-tsdoc-tags.js && api-extractor run --local --verbose",
"api:verify": "node scripts/fix-tsdoc-tags.js && api-extractor run",
"build": "cross-env-shell NODE_ENV=prod SASS_PATH=node_modules \"stencil build --config stencil.config.dist.ts\"",
"api:update": "npm run build && node scripts/fix-tsdoc-tags.js && api-extractor run --local --verbose",
"api:verify": "npm run build && node scripts/fix-tsdoc-tags.js && api-extractor run",
"build": "cross-env-shell NODE_ENV=prod SASS_PATH=node_modules \"stencil build --config stencil.config.dist.ts\"",
🤖 Prompt for AI Agents
In package.json around lines 19 to 21, the "api:verify" script runs the
tag-fixer and API Extractor but doesn’t build types first, causing failures on
clean CI when dist/types is missing; change the script to run the build before
the fixer and extractor (i.e., prepend the same build step used by "api:update":
run the build command first, then node scripts/fix-tsdoc-tags.js, then
api-extractor run) so dist/types exists before verification.

🛠️ Refactor suggestion

If you migrate fix-tsdoc-tags to ESM (.mjs), update these script entries

Lint currently flags CommonJS usage in scripts/fix-tsdoc-tags.js. If you adopt the ESM rewrite proposed in the script’s review, update the script paths here accordingly.

ESM variant of the scripts section (only the relevant lines shown):

-    "api:update": "npm run build && node scripts/fix-tsdoc-tags.js && api-extractor run --local --verbose",
-    "api:verify": "npm run build && node scripts/fix-tsdoc-tags.js && api-extractor run",
+    "api:update": "npm run build && node scripts/fix-tsdoc-tags.mjs && api-extractor run --local --verbose",
+    "api:verify": "npm run build && node scripts/fix-tsdoc-tags.mjs && api-extractor run",

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In package.json around lines 19–21, the scripts still call
scripts/fix-tsdoc-tags.js (CommonJS); update the script entries to reference the
ESM build (scripts/fix-tsdoc-tags.mjs) instead of the .js file — e.g. replace
node scripts/fix-tsdoc-tags.js with node scripts/fix-tsdoc-tags.mjs in both
api:update and api:verify (and update any other package.json/CI script
references to the .mjs path).

Comment on lines +45 to +46
"@stencil/core": "^4.36.3",
"@stencil/sass": "^3.2.2",
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Stencil v4 upgrade: verify TypeScript and Jest compatibility matrix

You’ve bumped @stencil/core to ^4.36.3 and @stencil/sass to ^3.2.2 while keeping typescript at ^4.9.5 and Jest deps at ^27.x. Stencil 4 typically expects TS 5.x and pairs with a newer Jest via stencil test. Mismatches can lead to subtle type or test-runner issues. Please confirm supported ranges for your node/TS/Jest toolchain and adjust if needed (e.g., bump to TS 5.x).

Run this script to verify peer requirements and local versions:


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Node version:"
node -v

echo -e "\nStencil core version:"
npm view @stencil/core version

echo -e "\nStencil peerDependencies (for TS/Jest expectations):"
npm view @stencil/core peerDependencies || true

echo -e "\nLocal devDependency TypeScript:"
jq -r '.devDependencies.typescript // "N/A"' package.json

echo -e "\nLocal Jest versions:"
jq -r '.devDependencies.jest, .devDependencies["jest-cli"]' package.json

echo -e "\nStencil test runner resolves Jest itself; ensure no global Jest conflicts are introduced."

Length of output: 825


Bump TypeScript devDependency to 5.x to align with Stencil v4

Stencil v4 ships with and tests against TypeScript 5.x (e.g. v5.5.3 in v4.20.0) and aims to stay within one minor version of the latest TS release. Using TS 4.9.5 falls outside this range and can lead to unexpected compilation or type-checking errors—please update your devDependencies.typescript to a 5.x version (for example: "typescript": "^5.5.3"). (stenciljs.com)

Stencil v4’s test runner supports Jest v24–29, so your existing "jest": "^27.5.1" can remain unchanged. (stenciljs.com)

🤖 Prompt for AI Agents
In package.json around lines 45-46, the devDependency for TypeScript is
currently 4.x which is incompatible with Stencil v4; update
devDependencies.typescript to a 5.x release (for example "typescript": "^5.5.3")
to align with Stencil v4’s supported TypeScript range, then run npm install (or
yarn) and re-run the build/type-check to verify no new type errors appear.

Comment on lines 8 to 11
const fs = require('fs');
const path = require('path');
const glob = require('glob');

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

⚠️ Potential issue

Lint failures: switch to ESM or adjust imports to address rules

ESLint flags CommonJS require and prefers Node built-ins via node: specifiers. Two-step plan:

  • Minimum change: use node: specifiers now; remove __dirname usage (see next comment); add missing JSDoc.
  • Preferred: migrate the script to ESM (.mjs) and update package.json scripts.

Minimum change patch for this file:

-const fs = require('fs');
-const path = require('path');
-const glob = require('glob');
+const fs = require('node:fs');
+const path = require('node:path');
+const glob = require('glob');

The change below removes __dirname usage and adds JSDoc; see follow-up comments.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const fs = require('fs');
const path = require('path');
const glob = require('glob');
// scripts/fix-tsdoc-tags.js
// … other imports or code above …
-const fs = require('fs');
-const path = require('path');
const fs = require('node:fs');
const path = require('node:path');
const glob = require('glob');
// … rest of the script …
🧰 Tools
🪛 GitHub Check: Lint

[failure] 10-10:
Do not use "require"


[failure] 10-10:
A require() style import is forbidden


[failure] 9-9:
Prefer node:path over path


[failure] 9-9:
Do not use "require"


[failure] 9-9:
A require() style import is forbidden


[failure] 8-8:
Prefer node:fs over fs


[failure] 8-8:
Do not use "require"


[failure] 8-8:
A require() style import is forbidden

🤖 Prompt for AI Agents
In scripts/fix-tsdoc-tags.js around lines 8 to 11, ESLint flags the CommonJS
requires and suggests node: specifiers and removal of __dirname; update the top
imports to use Node built-in specifiers (require('node:fs'),
require('node:path'), require('node:glob')) and add a brief JSDoc block
describing the script's purpose, inputs, and exports; also remove any use of
__dirname elsewhere in the file (use path.resolve(process.cwd(), ...) or
similar) to satisfy the lint rule; optionally note that full migration to ESM
(.mjs + package.json scripts) is preferred but apply the above minimal changes
now.

Comment on lines 12 to 14
const DIST_TYPES_DIR = path.join(__dirname, '../dist/types');
const TEMP_TYPES_DIR = path.join(__dirname, '../temp/types');

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Avoid __dirname per lint; resolve from repo root instead

Derive paths from process.cwd() so the script can run from the repo root (as your npm scripts do) and pass the “no __dirname” rule.

-const DIST_TYPES_DIR = path.join(__dirname, '../dist/types');
-const TEMP_TYPES_DIR = path.join(__dirname, '../temp/types');
+const ROOT = process.cwd();
+const DIST_TYPES_DIR = path.resolve(ROOT, 'dist/types');
+const TEMP_TYPES_DIR = path.resolve(ROOT, 'temp/types');
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const DIST_TYPES_DIR = path.join(__dirname, '../dist/types');
const TEMP_TYPES_DIR = path.join(__dirname, '../temp/types');
const ROOT = process.cwd();
const DIST_TYPES_DIR = path.resolve(ROOT, 'dist/types');
const TEMP_TYPES_DIR = path.resolve(ROOT, 'temp/types');
🧰 Tools
🪛 GitHub Check: Lint

[failure] 13-13:
Do not use "__dirname"


[failure] 12-12:
Do not use "__dirname"

🤖 Prompt for AI Agents
In scripts/fix-tsdoc-tags.js around lines 12 to 14, the script currently builds
DIST_TYPES_DIR and TEMP_TYPES_DIR using __dirname which violates the lint rule
and prevents running from the repo root; change the path construction to derive
from process.cwd() (or path.resolve(process.cwd(), ...)) so the constants point
to the repo-root relative directories (e.g. resolve 'dist/types' and
'temp/types' from process.cwd()) ensuring the script runs from npm scripts and
complies with the no __dirname rule.

Comment on lines 15 to 31
function convertJSDocToTSDoc(content) {
let converted = content;

// Convert @default to @defaultValue with proper backtick formatting
// JSDoc: @default {prop: value} or @default value
// TSDoc: @defaultValue `{prop: value}` or @defaultValue `value`
converted = converted.replace(
/(@default\s+)(.+?)(?=\n|\*\/|$)/g,
(_fullMatch, _prefix, value) => {
const trimmedValue = value.trim();
// If the value is already wrapped in backticks, don't double-wrap
if (trimmedValue.startsWith('`') && trimmedValue.endsWith('`')) {
return `@defaultValue ${trimmedValue}`;
}
return `@defaultValue \`${trimmedValue}\``;
}
);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Add JSDoc to satisfy lint and document edge cases in @default conversion

Document the behavior and constraints of convertJSDocToTSDoc, especially backtick handling and one-line limitation of the regex.

-function convertJSDocToTSDoc(content) {
+/**
+ * Convert common JSDoc tags to TSDoc-compatible tags for API Extractor consumption.
+ *
+ * - @default -> @defaultValue, wrapping the value in backticks if not already.
+ *   Note: The regex is line-oriented and won’t capture multi-line default values.
+ * - @private -> @internal
+ *
+ * Tags like @param, @returns, @see, @deprecated, and @example are left as-is.
+ *
+ * @param {string} content - The original .d.ts file content.
+ * @returns {string} The transformed content.
+ */
+function convertJSDocToTSDoc(content) {
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
function convertJSDocToTSDoc(content) {
let converted = content;
// Convert @default to @defaultValue with proper backtick formatting
// JSDoc: @default {prop: value} or @default value
// TSDoc: @defaultValue `{prop: value}` or @defaultValue `value`
converted = converted.replace(
/(@default\s+)(.+?)(?=\n|\*\/|$)/g,
(_fullMatch, _prefix, value) => {
const trimmedValue = value.trim();
// If the value is already wrapped in backticks, don't double-wrap
if (trimmedValue.startsWith('`') && trimmedValue.endsWith('`')) {
return `@defaultValue ${trimmedValue}`;
}
return `@defaultValue \`${trimmedValue}\``;
}
);
/**
* Convert common JSDoc tags to TSDoc-compatible tags for API Extractor consumption.
*
* - @default -> @defaultValue, wrapping the value in backticks if not already.
* Note: The regex is line-oriented and won’t capture multi-line default values.
* - @private -> @internal
*
* Tags like @param, @returns, @see, @deprecated, and @example are left as-is.
*
* @param {string} content - The original .d.ts file content.
* @returns {string} The transformed content.
*/
function convertJSDocToTSDoc(content) {
let converted = content;
// Convert @default to @defaultValue with proper backtick formatting
// JSDoc: @default {prop: value} or @default value
// TSDoc: @defaultValue `{prop: value}` or @defaultValue `value`
converted = converted.replace(
/(@default\s+)(.+?)(?=\n|\*\/|$)/g,
(_fullMatch, _prefix, value) => {
const trimmedValue = value.trim();
// If the value is already wrapped in backticks, don't double-wrap
if (trimmedValue.startsWith('`') && trimmedValue.endsWith('`')) {
return `@defaultValue ${trimmedValue}`;
}
return `@defaultValue \`${trimmedValue}\``;
}
);
// …rest of function…
}
🧰 Tools
🪛 GitHub Check: Lint

[warning] 15-15:
Missing JSDoc comment

🤖 Prompt for AI Agents
In scripts/fix-tsdoc-tags.js around lines 15 to 31, add a JSDoc block above
convertJSDocToTSDoc that documents its behavior and edge cases: explain that
@default is converted to @defaultValue and values are wrapped in backticks
unless already backticked, that the current implementation only handles
single-line @default entries due to the regex (it will not match multi-line or
block object values), and note any assumptions/limitations (e.g., whitespace
trimming, no escaping of backticks). Keep the comment concise and concrete so
linters and future maintainers understand the backtick handling and one-line
regex limitation.

Comment on lines +1 to +7
export interface LinkMarkAttrs {
href: string;
title: string | null;
target: string | null;
rel: string | null;
referrerpolicy: string | null;
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick (assertive)

Add TSDoc for API extractor and external consumers

This new public type should have TSDoc so it shows up correctly in generated docs and to make intent clear (especially that keys mirror DOM attribute names).

Apply this diff:

+/**
+ * Attributes for the ProseMirror `link` mark.
+ * Keys intentionally mirror DOM attribute names; use `null` to omit an attribute.
+ * @public
+ */
 export interface LinkMarkAttrs {
     href: string;
     title: string | null;
     target: string | null;
     rel: string | null;
     referrerpolicy: string | null;
 }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export interface LinkMarkAttrs {
href: string;
title: string | null;
target: string | null;
rel: string | null;
referrerpolicy: string | null;
}
/**
* Attributes for the ProseMirror `link` mark.
* Keys intentionally mirror DOM attribute names; use `null` to omit an attribute.
* @public
*/
export interface LinkMarkAttrs {
href: string;
title: string | null;
target: string | null;
rel: string | null;
referrerpolicy: string | null;
}
🤖 Prompt for AI Agents
In
src/components/text-editor/prosemirror-adapter/plugins/link/link-mark.types.ts
around lines 1 to 7, the exported LinkMarkAttrs interface lacks TSDoc for API
extractor and consumers; add a top-level TSDoc comment for the interface stating
it is a public type used for link mark attributes and that its keys mirror DOM
link attribute names, then add brief TSDoc comments for each property (href,
title, target, rel, referrerpolicy) describing expected values and nullability
so generated docs clearly convey intent.

Comment on lines +3 to +22
// Mock ResizeObserver for tests
if (!global.ResizeObserver) {
global.ResizeObserver = class ResizeObserver {
constructor(_callback: ResizeObserverCallback) {
// Mock implementation - just store the callback
}

observe(_target: Element): void {
// Mock implementation
}

unobserve(_target: Element): void {
// Mock implementation
}

disconnect(): void {
// Mock implementation
}
};
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Use globalThis (typed) instead of global to avoid TS typing issues and ensure cross-env safety.

global.ResizeObserver can cause TS type errors (property not on NodeJS.Global) and is less portable. Prefer globalThis, and feature-detect with typeof:

-// Mock ResizeObserver for tests
-if (!global.ResizeObserver) {
-    global.ResizeObserver = class ResizeObserver {
+// Mock ResizeObserver for tests
+const g: any = globalThis as any;
+if (typeof g.ResizeObserver === 'undefined') {
+    g.ResizeObserver = class ResizeObserver {
         constructor(_callback: ResizeObserverCallback) {
             // Mock implementation - just store the callback
         }
 
         observe(_target: Element): void {
             // Mock implementation
         }
 
         unobserve(_target: Element): void {
             // Mock implementation
         }
 
         disconnect(): void {
             // Mock implementation
         }
     };
 }

If desired for slightly more realistic behavior, you could queue the callback once on observe() with a zero-sized entry, but keep it a no-op unless tests need it.

Search for other direct uses of global. that might also want globalThis:


🏁 Script executed:

#!/bin/bash
rg -nP '\bglobal\.' -g '!**/node_modules/**' -C2

Length of output: 1595


Migrate all global.* usages to globalThis in test setup and specs

We’ve identified additional direct uses of global.* in your test files—these should be updated to use globalThis (typed) for proper TS support and cross-environment safety.

Files to update:

  • src/test-setup.ts (already noted)
  • src/util/files.spec.ts (mocking crypto)
  • src/components/dock/dock-button/dock-button.spec.tsx (mocking IntersectionObserver)
  • src/components/action-bar/action-bar.spec.tsx (mocking IntersectionObserver)

Suggested refactors:

--- a/src/util/files.spec.ts
+++ b/src/util/files.spec.ts
@@ -4,7 +4,11 @@ beforeAll(() => {
     // Mock Crypto
-    if (global.crypto === undefined) {
-        (global.crypto as Partial<Crypto>) = {
+    const g: any = globalThis as any;
+    if (typeof g.crypto === 'undefined') {
+        g.crypto = {
             randomUUID: jest.fn(
                 () => '12345678-1234-1234-1234-123456789abc'
             ),
+        } as Partial<Crypto>;
     };
 });
--- a/src/components/dock/dock-button/dock-button.spec.tsx
+++ b/src/components/dock/dock-button/dock-button.spec.tsx
@@ -13,7 +13,9 @@ describe('DockButton', () => {
     // ...
-    global.IntersectionObserver = IntersectionObserverMock as any;
+    const g: any = globalThis as any;
+    // Replace native IntersectionObserver in JSDOM
+    g.IntersectionObserver = IntersectionObserverMock as any;

     beforeEach(async () => {
         page = await newSpecPage({
--- a/src/components/action-bar/action-bar.spec.tsx
+++ b/src/components/action-bar/action-bar.spec.tsx
@@ -40,7 +40,9 @@ describe('ActionBar', () => {
         return intersectionObserverMockedFunctions;
     }) as any;
-    global.IntersectionObserver = intersectionObserverMock;
+    const g: any = globalThis as any;
+    // Override for test isolation
+    g.IntersectionObserver = intersectionObserverMock;

     beforeEach(async () => {
         page = await newSpecPage({

And in src/test-setup.ts, replace your existing global.ResizeObserver mock with:

--- a/src/test-setup.ts
+++ b/src/test-setup.ts
@@ -1,4 +1,6 @@
 // Mock ResizeObserver for tests
-// (existing global.ResizeObserver code)
+const g: any = globalThis as any;
+if (typeof g.ResizeObserver === 'undefined') {
     g.ResizeObserver = class ResizeObserver {
         constructor(_callback: ResizeObserverCallback) {
             // Mock implementation...

These changes will eliminate TS errors around global and ensure compatibility across Node, JSDOM, and browser-like test environments.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In src/test-setup.ts around lines 3–22, the ResizeObserver mock uses
global.ResizeObserver; replace all global.* usage with typed globalThis.* to
ensure TS and cross-environment safety — e.g., set globalThis.ResizeObserver to
your mock class and add or update a declaration for globalThis (or cast to
unknown as needed) so TypeScript recognizes ResizeObserver on globalThis; also
apply the same pattern in src/util/files.spec.ts,
src/components/dock/dock-button/dock-button.spec.tsx and
src/components/action-bar/action-bar.spec.tsx where crypto or
IntersectionObserver are mocked (use globalThis.crypto and
globalThis.IntersectionObserver with appropriate typings or declare
module/globalThis augmentations).

Comment on lines +16 to +20
plugins: [
sass({
includePaths: ['node_modules'],
}),
],
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Switching Sass plugin to include node_modules is sensible; verify no remaining non-relative imports from src/style

This change is correct for resolving third-party Sass packages. Please also confirm that no SCSS still relies on old absolute-like paths (e.g., @use "style/..."), since includePaths now only whitelists node_modules and we converted internal imports to be relative.

Run this to surface any remaining problematic imports:


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "Looking for SCSS @use/@import statements that reference src/style without a relative prefix..."
rg -nPI --type=scss -C1 $'@(?:use|import)\\s+[\'"](?:style/|src/style/)' src || true

echo
echo "Looking for deprecated @import usage (should be migrated to @use/@forward)..."
rg -nPI --type=scss -C1 '^\s*@import\s+' src || true

echo
echo "If the first search returns no results and the second is empty or only third-party imports, you're good."

Length of output: 819


I’ll rerun the searches without relying on ripgrep’s built-in SCSS type:


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "Looking for SCSS @use/@import statements that reference src/style without a relative prefix..."
rg -nP -g '*.scss' -C1 $'@(?:use|import)\\s+[\\'\"](?:style/|src/style/)' src || true

echo
echo "Looking for deprecated @import usage (should be migrated to @use/@forward)..."
rg -nP -g '*.scss' -C1 '^\s*@import\s+' src || true

echo
echo "If the first search returns no results and the second is empty or only third-party imports, you’re good."

Length of output: 328


Let’s switch to a plain find+grep approach to avoid the quoting issues:


🏁 Script executed:

#!/usr/bin/env bash
set -euo pipefail

echo "Looking for SCSS @use/@import referencing src/style without a relative prefix..."
find src -type f -name '*.scss' -exec grep -nE "@(use|import)\s+['\"](style/|src/style/)" {} \; || true

echo
echo "Looking for deprecated @import usage (should be migrated to @use/@forward)..."
find src -type f -name '*.scss' -exec grep -nE "^\s*@import\s+" {} \; || true

echo
echo "If the first search returns no results and the second is empty or only third-party imports, you’re good."

Length of output: 3919


SCSS imports verification: absolute paths cleared, deprecated @import remains

I confirmed that there are no remaining @use/@import statements pulling from style/ or src/style/ without a relative prefix—good work there. However, the scan surfaced a large number of internal SCSS files still using the deprecated @import syntax rather than the recommended @use/@forward module system. These should be migrated to ensure compatibility with Dart Sass and future-proof our styles.

Files/areas needing refactor (examples):

  • src/**/*.scss importing partials under ./partial-styles/
  • src/**/*.scss importing root-level styles like ./application
  • src/**/*.scss importing sibling components (e.g. ../checkbox/checkbox.scss, ../radio-button-group/radio-button.scss)

Please replace those internal @import './…' statements with @use and @forward directives. Once migrated, rerun the grep check to confirm no leftover @import of internal SCSS.

🤖 Prompt for AI Agents
In stencil.config.dist.ts around lines 16 to 20, the config is fine but the
review found many internal SCSS files across src/ still using deprecated @import
for local partials; replace internal @import './…' usages with the Sass module
system by converting component and root-level style imports to @use and @forward
(create index modules where appropriate for grouped forwards), update relative
paths to use module namespaces, and then run the grep scan to verify no
remaining internal @import occurrences.

Comment on lines +11 to +15
plugins: [
sass({
includePaths: ['node_modules'],
}),
],
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

❓ Verification inconclusive

Sass includePaths looks good; verify parity across all configs to avoid environment drift.

Using sass({ includePaths: ['node_modules'] }) avoids brittle relative imports from third-party packages. Ensure the same option is applied in stencil.config.dist.ts and stencil.config.docs.ts so Sass resolution is consistent between dev, dist, and docs builds.

Run:


🏁 Script executed:

#!/bin/bash
rg -nP "sass\\(\\{\\s*includePaths:\\s*\\['node_modules'\\]\\s*\\}\\)" stencil.config*.ts -n

Length of output: 136


Ensure Sass includePaths Consistency Across All Stencil Configs
Our search didn’t find the sass({ includePaths: ['node_modules'] }) invocation in the distribution or docs configs, which risks environment drift between your dev, dist, and docs builds. Please add the same plugin configuration to both:

stencil.config.dist.ts
stencil.config.docs.ts

Suggested diff for each:

 plugins: [
-   /* existing plugins… */
+   sass({
+     includePaths: ['node_modules'],
+   }),
    /* existing plugins… */
 ],

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents
In stencil.config.dist.ts and stencil.config.docs.ts (add to their plugins
arrays analogous to stencil.config.ts lines 11–15), add the sass plugin
invocation with includePaths: ['node_modules'] to match the dev config; also
ensure the top of each file imports the sass plugin if missing (e.g., import {
sass } from '@stencil/sass' or the project’s existing sass package) and place
the plugin entry in the plugins array exactly as: sass({ includePaths:
['node_modules'] }) so dist/docs builds use the same Sass resolution as dev.

Comment on lines +29 to +33
],
"supportForTags": {
"@exampleComponent": true,
"@note": true
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Ensure all used tags are supported, or add support to avoid extractor warnings

You’ve declared @note and @exampleComponent in supportForTags. If @slot and @sourceFile (both defined above) appear widely, enabling them here can avoid “Unsupported tag” diagnostics depending on parser settings. Also, you removed custom definitions for @id, @event, @emits. If those tags still exist in comments anywhere, API Extractor will warn/error.

Add the extra supported tags:

   "supportForTags": {
-    "@exampleComponent": true,
-    "@note": true
+    "@exampleComponent": true,
+    "@note": true,
+    "@slot": true,
+    "@sourceFile": true
   }

Scan for removed/unknown tags:


🏁 Script executed:

#!/bin/bash
# Find usages of removed or potentially unsupported tags
rg -nP --glob '!*dist*' --glob '!**/temp/**' '@(id|event|emits)\b|@private\b' src/ || true

# Also check generated types that will be fed to API Extractor
rg -nP '@(id|event|emits)\b|@private\b' dist/types/ || true

Length of output: 3305


🏁 Script executed:

#!/bin/bash
# Display the first 200 lines of tsdoc.json to inspect tagDefinitions and supportForTags
sed -n '1,200p' tsdoc.json

Length of output: 761


Add missing supportForTags entries for all defined TSDoc tags to prevent extractor warnings

  • We found that @slot, @sourceFile, and especially @private (used widely across your source) are defined in tagDefinitions but not yet enabled in supportForTags. Without these entries, API Extractor will emit “Unsupported tag” diagnostics for each occurrence.
  • Our ripgrep scan showed no usages of @id, @event, or @emits in src/, so removing those definitions is safe.

Apply this diff to tsdoc.json:

   "supportForTags": {
-    "@exampleComponent": true,
-    "@note": true
+    "@exampleComponent": true,
+    "@note": true,
+    "@slot": true,
+    "@sourceFile": true,
+    "@private": true
   }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
],
"supportForTags": {
"@exampleComponent": true,
"@note": true
}
],
"supportForTags": {
"@exampleComponent": true,
"@note": true,
"@slot": true,
"@sourceFile": true,
"@private": true
}
🤖 Prompt for AI Agents
In tsdoc.json around lines 29 to 33, supportForTags is missing entries for
several tags declared in tagDefinitions (notably @slot, @sourceFile, and
@private) which causes API Extractor "Unsupported tag" diagnostics; update
supportForTags to include those tags with true values (e.g., "@slot": true,
"@sourceFile": true, "@private": true) and remove unused tags that your scan
showed absent (such as "@id", "@event", and "@emits") so supportForTags matches
the actual tagDefinitions and usage in src/.

@adrianschmidt adrianschmidt marked this pull request as draft August 25, 2025 06:49
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