-
Notifications
You must be signed in to change notification settings - Fork 14
refactor: astro stencil showcase #5879
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR refactors the stencil-showcase from an Angular-based implementation to an Astro-based implementation. This is a significant architectural change that enables web components to be rendered with Astro's static site generation capabilities.
Changes:
- Migrated stencil-showcase from Angular to Astro framework
- Added Mitosis plugin for Astro code generation supporting hybrid rendering (static + web components)
- Fixed boolean string handling in utility functions to properly parse 'true'/'false' strings for HTML attribute compatibility
- Added new component properties and lifecycle improvements (open prop for accordion-item, hideSubNavigation for navigation-item)
- Updated Stencil build configuration to generate bundled custom elements for easier Astro integration
Reviewed changes
Copilot reviewed 88 out of 92 changed files in this pull request and generated 23 comments.
Show a summary per file
| File | Description |
|---|---|
| xo.config.js | Excludes stencil-showcase from linting |
| showcases/stencil-showcase/tsconfig.json | Replaces Angular config with Astro config, points to output/astro |
| showcases/stencil-showcase/package.json | Replaces Angular build scripts with Astro, updates dependencies |
| showcases/stencil-showcase/astro.config.mjs | New Astro configuration with Vite plugin for copying Stencil bundle |
| showcases/stencil-showcase/src/**/*.astro | New Astro component pages replacing Angular components |
| packages/components/src/utils/index.ts | Fixed boolean string parsing to check 'true'/'false' explicitly |
| packages/components/src/components/*/ | Various component updates for Astro compatibility (page, header, navigation-item, drawer, custom-select, accordion, accordion-item) |
| packages/components/configs/plugins/astro/ | New Mitosis plugin for generating Astro components |
| packages/components/configs/mitosis.config.cjs | Added Astro as compilation target |
| packages/components/scripts/post-build/stencil.ts | Added logic to remove duplicate Astro-rendered elements on hydration |
| output/stencil/stencil.config.ts | Added bundled custom elements output target |
| showcases/e2e/default.ts | Updated routing logic to handle Astro's non-hash routing |
| showcases/e2e/header/*.spec.ts | Removed stencil skip conditions for header tests |
Comments suppressed due to low confidence (1)
packages/components/src/utils/index.ts:111
- The change from checking
Boolean(originBool)to explicitly checkingoriginBool === 'true'for string values is significant and could break existing code.
The old behavior: Boolean('false') returns true (any non-empty string is truthy)
The new behavior: 'false' === 'true' returns false
This is actually a bug fix, but it's a breaking change. Any code that was passing the string 'false' and expecting it to be treated as truthy will now be treated as falsy.
Verify that:
- All usages in the codebase that pass boolean string values are updated
- This change is documented as potentially breaking
- Framework integrations (especially Astro with HTML attributes) correctly pass 'true'/'false' strings
Additionally, consider whether strings other than 'true' should be treated as false (e.g., should empty string '' return false or undefined?).
export const getBoolean = (
originBool?: boolean | string,
propertyName?: string
): boolean | undefined => {
if (originBool === undefined || originBool === null) return;
if (typeof originBool === 'string') {
return Boolean(propertyName === originBool || originBool === 'true');
}
return Boolean(originBool);
};
packages/components/src/components/custom-select/custom-select.lite.tsx
Outdated
Show resolved
Hide resolved
packages/components/src/components/custom-select/custom-select.lite.tsx
Outdated
Show resolved
Hide resolved
packages/components/src/components/accordion/accordion.lite.tsx
Outdated
Show resolved
Hide resolved
packages/components/src/components/custom-select/custom-select.lite.tsx
Outdated
Show resolved
Hide resolved
packages/components/src/components/navigation-item/navigation-item.lite.tsx
Show resolved
Hide resolved
packages/components/src/components/accordion-item/accordion-item.lite.tsx
Show resolved
Hide resolved
packages/components/src/components/navigation-item/navigation-item.lite.tsx
Show resolved
Hide resolved
… into refactor-astro-stencil-showcase
…l-showcase # Conflicts: # package-lock.json # showcases/stencil-showcase/package.json
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
…-astro-stencil-showcase
# Conflicts: # .config/cspellignorewords.txt
Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Proposed changes
resolves # (issue number)
Types of changes
Further comments
🔭🐙🐈 Test this branch here: https://design-system.deutschebahn.com/core-web/review/refactor-astro-stencil-showcase