Skip to content

Conversation

@adrianschmidt
Copy link
Contributor

@adrianschmidt adrianschmidt commented Jan 19, 2026

Summary by CodeRabbit

  • Bug Fixes

    • Safer date-picker initialization to avoid runtime errors in environments missing certain visibility checks.
  • Chores

    • Upgraded framework and Sass; improved build/verification pipeline with a preprocessing step to adjust doc tags.
    • Adjusted build tooling to resolve Sass imports from node_modules and updated test runner behavior for CI/debug.
  • Tests

    • Replaced fragile snapshots with DOM-based assertions and added test setup for ResizeObserver.
  • Documentation

    • Updated doc/tsdoc and extractor reporting to align with new preprocessing.

✏️ Tip: You can customize this high-level summary in your review settings.

fix: #3653

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

`TextEditorPlugin` is literally only found here when searching
the whole of our GitHub org:
https://github.com/search?q=org%3ALundalogik%20TextEditorPlugin&type=code
@adrianschmidt adrianschmidt requested a review from a team as a code owner January 19, 2026 14:38
@coderabbitai
Copy link

coderabbitai bot commented Jan 19, 2026

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

Build and type-extraction pipeline updated for Stencil 4: dependency bumps, added a JSDoc→TSDoc conversion script that copies dist/typestemp/types, API Extractor config switched to temp/types, Sass includePaths added, scattered SCSS @use imports, and several small type/accessibility refactors across components and tests.

Changes

Cohort / File(s) Summary
API Extractor & TSDoc config
api-extractor.json, tsdoc.json
Switched mainEntryPointFilePath and compiler.overrideTsconfig.include from dist/types/... to temp/types/...; added ae-extra-release-tag extractor message rule; tsdoc.json now extends api-extractor base, adjusts tagDefinitions and supportForTags.
Package scripts & build pipeline
package.json, scripts/fix-tsdoc-tags.cjs, scripts/run-tests.cjs
Added fix-tsdoc-tags.cjs to copy/process dist/typestemp/types converting @default@defaultValue and @private@internal; package scripts api:update/api:verify updated to run this step; added run-tests.cjs wrapper controlling CI/workers and verbose flags.
Stencil configs & test setup
stencil.config.ts, stencil.config.dist.ts, stencil.config.docs.ts, src/test-setup.ts
Sass plugin configured with includePaths: ['node_modules']; test config adds setupFilesAfterEnv: ['<rootDir>/src/test-setup.ts']; new global ResizeObserver mock added for tests.
SCSS module imports
src/components/.../partial-styles/* (multiple)
Added @use imports for functions, mixins, or shared style modules across many component partial styles to enable utility functions and variables.
Text-editor type refactor
src/components/text-editor/prosemirror-adapter/plugins/link/*, src/components/text-editor/types.ts, src/components/text-editor/prosemirror-adapter/prosemirror-adapter.tsx
Extracted LinkMarkAttrs into new link-mark.types.ts and updated imports; removed exported TextEditorPlugin type from types.ts.
Component visibility & minor fixes
src/components/checkbox/checkbox.tsx, src/components/dock/dock-button/dock-button.tsx, src/components/portal/portal.tsx, src/components/date-picker/.../flatpickr-adapter.tsx
Several watcher methods changed from protectedpublic; flattened optional chaining for checkVisibility call to avoid runtime errors.
Tests & snapshots
src/components/split-button/split-button.spec.tsx, src/components/color-picker/color-picker.spec.tsx
Replaced snapshot assertions with DOM structure checks in split-button tests; updated color-picker snapshot expectations for shadow root attribute.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant DevScript as npm script (api:update / api:verify)
  participant Build as Stencil Build
  participant FS as File System (dist/types)
  participant Fix as fix-tsdoc-tags.cjs
  participant TempFS as File System (temp/types)
  participant ApiExtractor as API Extractor

  rect rgba(200,200,255,0.5)
  DevScript->>Build: run build (may run twice if missing)
  Build-->>FS: emit .d.ts files to dist/types
  end

  rect rgba(200,255,200,0.5)
  DevScript->>Fix: invoke fix-tsdoc-tags.cjs
  Fix->>FS: read *.d.ts
  Fix->>Fix: convert `@default`→@defaultValue, `@private`→@internal
  Fix->>TempFS: write processed files to temp/types
  end

  rect rgba(255,200,200,0.5)
  DevScript->>ApiExtractor: run api-extractor against temp/types
  ApiExtractor->>TempFS: read temp/types/index.d.ts
  ApiExtractor-->>DevScript: produce api report / warnings
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~22 minutes

Possibly related PRs

Suggested labels

3rd-party, maintenance

Suggested reviewers

  • jgroth
  • Kiarokh
🚥 Pre-merge checks | ✅ 3 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 20.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Update Stencil' is vague and generic, failing to convey specific details about the changeset despite significant updates including Stencil v4 migration and API extraction preprocessing. Consider a more descriptive title such as 'build: upgrade to Stencil v4 and add TSDoc preprocessing for API extraction' to better reflect the scope and primary changes.
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check ✅ Passed The changeset comprehensively addresses issue #3653 by upgrading to Stencil v4, updating build/test configurations, adding TSDoc preprocessing for API extraction, and removing unused code.
Out of Scope Changes check ✅ Passed All changes are directly related to the Stencil v4 upgrade objectives: SCSS imports for modules, watcher visibility updates aligned with Stencil v4 requirements, test infrastructure updates, and API extraction preprocessing.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
🧪 Generate unit tests (beta)
  • 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

Comment @coderabbitai help to get the list of available commands and usage tips.

@github-actions
Copy link

Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3782/

coderabbitai[bot]

This comment was marked as resolved.

…ed changes

Includes v4 migration changes:
- test environment setup with ResizeObserver mock,
- SASS plugin configuration updates, and
- formatting adjustments.
@adrianschmidt adrianschmidt force-pushed the update-stencil branch 6 times, most recently from 321d901 to ea195e7 Compare January 20, 2026 14:50
@adrianschmidt
Copy link
Contributor Author

@coderabbitai Full review

@coderabbitai
Copy link

coderabbitai bot commented Jan 20, 2026

✅ Actions performed

Full review triggered.

@adrianschmidt adrianschmidt changed the title Update Stencil Update to Stencil 4 Jan 20, 2026
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: 5

🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 19-20: The package.json script "api:verify" uses POSIX-only `test
-f` which fails on Windows; update the "api:verify" script to use the
cross-platform utility already in devDependencies (replace `test -f
src/components.d.ts` with `shx test -f src/components.d.ts`) or alternatively
replace that check with a tiny Node invocation (e.g., `node -e` checking
fs.existsSync('src/components.d.ts')) so the conditional before `npm run build`
works on all platforms; modify the "api:verify" command accordingly while
leaving "api:update" unchanged.
- Around line 46-47: Update package.json to bump the TypeScript devDependency
from ^4.9.5 to ^5.0.0 (or ^5.x) so it meets Stencil v4.36.3's requirement and
ensure any related toolchain constraints in devDependencies (e.g., `@types/`,
tsconfig-related packages) are compatible; also add an "engines" field (e.g.,
"engines": { "node": ">=16" }) to enforce Node.js 16+ for the project toolchain.
Locate the "typescript" entry in devDependencies and replace its version, and
add the top-level "engines" object to package.json to enforce the Node version.

In `@scripts/fix-tsdoc-tags.cjs`:
- Around line 8-11: The script requires the npm package via the statement const
glob = require('glob'); but glob is not declared as a direct dependency,
creating a fragile transitive dependency used during build tasks (api:update,
api:verify); fix this by adding glob to package.json under devDependencies (or
running npm/yarn add --dev glob to update lockfile) so the script reliably
resolves in CI, or alternatively refactor the script to remove the
require('glob') usage and implement directory traversal with built-in fs/path
APIs if you prefer not to add the dependency.

In `@scripts/run-tests.cjs`:
- Around line 46-54: The current exit handling always falls back to 0 when
spawnSync yields a null status, which hides signal-based terminations; update
the termination logic after spawnSync to: if result.status is a number call
process.exit(result.status); else if result.signal is set re-emit the same
signal against the current process (e.g., process.kill(process.pid,
result.signal)) so the node process terminates with the same signal; otherwise
fall back to a non-zero exit (e.g., process.exit(1)). Reference the spawnSync
call, result.status, result.signal and process.exit/process.kill to locate and
implement the change.

In `@src/components/date-picker/flatpickr-adapter/flatpickr-adapter.tsx`:
- Around line 165-167: The optional chaining on (this.container as
any).checkVisibility?.() can return undefined in browsers without
checkVisibility, causing the condition to wrongly bail out; update the check in
the method using this.isOpen and this.container to treat a missing
checkVisibility as "visible" by replacing the expression using optional chaining
with one that uses nullish coalescing (e.g., call checkVisibility() if present
otherwise default to true), so the condition becomes: if (!this.isOpen ||
!((this.container as any).checkVisibility?.() ?? true)) return; — locate this
logic around the flatpickr adapter initialization where this.isOpen and
this.container are referenced.

Converts Stencil-generated JSDoc tags to TSDoc format
(@default@DefaultValue, @Private@internal) by copying
types to `./temp/types` before API extraction. Ensures
compatibility with API Extractor's TSDoc requirements
…enabled

Adds a test runner wrapper script that automatically enables verbose
output when debug logging is enabled.

The wrapper detects ACTIONS_STEP_DEBUG, RUNNER_DEBUG, or DEBUG env vars
and adds the --verbose flag to the stencil test command.
Work around Stencil v4 e2e test parallelization issues that cause
random timeouts in CI but not locally. Changes only apply when CI=true.

- Add --disable-gpu browser arg in stencil.config.ts
- Add --max-workers=2 in run-tests.cjs

See: stenciljs/core#6157
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.

Update lime-elements to Stencil 4

2 participants