Update Stencil to v4, plus changes required by that update, mostly to config, tests, and other dev-env stuff#3791
Open
adrianschmidt wants to merge 2 commits intofix-flaky-testsfrom
Open
Update Stencil to v4, plus changes required by that update, mostly to config, tests, and other dev-env stuff#3791adrianschmidt wants to merge 2 commits intofix-flaky-testsfrom
adrianschmidt wants to merge 2 commits intofix-flaky-testsfrom
Conversation
📝 WalkthroughWalkthroughThis PR updates API Extractor configuration to use a temporary types directory, adds a TSDoc tag fixer script, upgrades Stencil from v3 to v4, changes several component method visibility modifiers from protected to public, refactors link mark types into a separate file, configures Sass plugin include paths, and adds test polyfills and configuration updates. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3791/ |
…ed changes Includes v4 migration changes: - test environment setup with ResizeObserver mock, - SASS plugin configuration updates, and - formatting adjustments.
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
adrianschmidt
force-pushed
the
update-stencil
branch
from
5f17bdc to
016276f
Compare
adrianschmidt
commented
Jan 20, 2026
Comment on lines
+9
to
+19
| const path = require('node:path'); | ||
|
|
||
| const stencilScript = require.resolve('@stencil/core/bin/stencil'); | ||
| const stencilScript = path.resolve( | ||
| __dirname, | ||
| '..', | ||
| 'node_modules', | ||
| '@stencil', | ||
| 'core', | ||
| 'bin', | ||
| 'stencil' | ||
| ); |
Contributor
Author
There was a problem hiding this comment.
Resolve the stencil CLI path directly (can't use require.resolve because Stencil v4's package.json exports field doesn't expose bin/stencil).
Contributor
Author
|
@coderabbitai full review |
✅ Actions performedFull review triggered. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@package.json`:
- Around line 19-20: The api:verify script currently short‑circuits when
src/components.d.ts exists and also runs the build twice on the fallback; change
the script so it always runs node scripts/fix-tsdoc-tags.cjs and api-extractor
run, and only invokes npm run build when the file is missing. Update the
"api:verify" entry to perform a shell conditional (if/then/else or grouped
condition) that checks shx test -f src/components.d.ts and runs either the
post-build steps (node scripts/fix-tsdoc-tags.cjs && api-extractor run) after a
build or just those post-build steps when the file already exists, ensuring no
duplicate npm run build and no short‑circuiting of fix-tsdoc-tags.cjs or
api-extractor.
In `@src/components/date-picker/flatpickr-adapter/flatpickr-adapter.tsx`:
- Around line 165-167: The current guard in the create/init path uses optional
chaining on (this.container as any).checkVisibility?.() which yields undefined
when the method is missing and causes an unintended early return; update the
check so you only return early if checkVisibility explicitly returns false
(i.e., call checkVisibility only when it exists and treat undefined as visible).
Locate the conditional around this.isOpen and (this.container as
any).checkVisibility in flatpickr-adapter.tsx and change it to call
checkVisibility only if present and test for === false (or use typeof to detect
the function) so creation proceeds when the API is unavailable.
Comment on lines
+19
to
+20
| "api:update": "npm run build && node scripts/fix-tsdoc-tags.cjs && api-extractor run --local --verbose", | ||
| "api:verify": "shx test -f src/components.d.ts || (npm run build && npm run build) && node scripts/fix-tsdoc-tags.cjs && api-extractor run", |
There was a problem hiding this comment.
Fix api:verify short‑circuiting and duplicate build.
At Line 20, || short‑circuits the rest of the script when src/components.d.ts exists, so fix-tsdoc-tags and api-extractor never run. Also the build runs twice on the fallback path.
✅ Proposed fix
- "api:verify": "shx test -f src/components.d.ts || (npm run build && npm run build) && node scripts/fix-tsdoc-tags.cjs && api-extractor run",
+ "api:verify": "(shx test -f src/components.d.ts || npm run build) && node scripts/fix-tsdoc-tags.cjs && 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.cjs && api-extractor run --local --verbose", | |
| "api:verify": "shx test -f src/components.d.ts || (npm run build && npm run build) && node scripts/fix-tsdoc-tags.cjs && api-extractor run", | |
| "api:update": "npm run build && node scripts/fix-tsdoc-tags.cjs && api-extractor run --local --verbose", | |
| "api:verify": "(shx test -f src/components.d.ts || npm run build) && node scripts/fix-tsdoc-tags.cjs && api-extractor run", |
🤖 Prompt for AI Agents
In `@package.json` around lines 19 - 20, The api:verify script currently
short‑circuits when src/components.d.ts exists and also runs the build twice on
the fallback; change the script so it always runs node
scripts/fix-tsdoc-tags.cjs and api-extractor run, and only invokes npm run build
when the file is missing. Update the "api:verify" entry to perform a shell
conditional (if/then/else or grouped condition) that checks shx test -f
src/components.d.ts and runs either the post-build steps (node
scripts/fix-tsdoc-tags.cjs && api-extractor run) after a build or just those
post-build steps when the file already exists, ensuring no duplicate npm run
build and no short‑circuiting of fix-tsdoc-tags.cjs or api-extractor.
Comment on lines
+165
to
167
| if (!this.isOpen || !(this.container as any).checkVisibility?.()) { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Optional chaining fallback breaks functionality when checkVisibility is unavailable.
When checkVisibility is not defined (e.g., older browsers, JSDOM test environments), checkVisibility?.() returns undefined. Negating undefined yields true, causing this method to return early and never create the flatpickr.
The intended behavior is likely: skip creation only if the element is explicitly not visible; if the API is unavailable, assume visible and proceed.
🐛 Proposed fix
- if (!this.isOpen || !(this.container as any).checkVisibility?.()) {
+ if (!this.isOpen || (this.container as any).checkVisibility?.() === false) {
return;
}This only returns early when checkVisibility explicitly returns false. If the method is undefined or returns true, the code proceeds normally.
🤖 Prompt for AI Agents
In `@src/components/date-picker/flatpickr-adapter/flatpickr-adapter.tsx` around
lines 165 - 167, The current guard in the create/init path uses optional
chaining on (this.container as any).checkVisibility?.() which yields undefined
when the method is missing and causes an unintended early return; update the
check so you only return early if checkVisibility explicitly returns false
(i.e., call checkVisibility only when it exists and treat undefined as visible).
Locate the conditional around this.isOpen and (this.container as
any).checkVisibility in flatpickr-adapter.tsx and change it to call
checkVisibility only if present and test for === false (or use typeof to detect
the function) so creation proceeds when the API is unavailable.
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
1 participant
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.
Summary by CodeRabbit
Chores
Tests
Refactoring
✏️ Tip: You can customize this high-level summary in your review settings.
Review:
Browsers tested:
(Check any that applies, it's ok to leave boxes unchecked if testing something didn't seem relevant.)
Windows:
Linux:
macOS:
Mobile: