-
Notifications
You must be signed in to change notification settings - Fork 4
Upgrade to Stencil 4 and add support for Typescript 4.6+ #150
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
Draft
adrianschmidt
wants to merge
16
commits into
main
Choose a base branch
from
upgrade-stencil
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Conversation
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
aeaea9c to
6ad0f24
Compare
This comment was marked as outdated.
This comment was marked as outdated.
13 tasks
f63eb70 to
f8e969d
Compare
f4b4926 to
b033d4a
Compare
54b9ff7 to
00f183a
Compare
00f183a to
958d449
Compare
4d102ba to
f7cbe0c
Compare
958d449 to
7abbec4
Compare
The @stencil/router package has been abandoned and does not support Stencil v4. After evaluating available alternatives, no functioning router libraries with Stencil v4 support were found, necessitating a custom implementation. Implementation: Built a hash-based routing system with three core components and domain-specific utilities: - kompendium-router: Top-level container component - kompendium-route-switch: Route container managing centralized navigation state and scroll behavior - kompendium-route: Individual route component with pattern matching and first-match routing logic (similar to React Router's Switch) - route-matching: Pure routing utilities for pattern matching and parameter extraction, supporting both required (:param) and optional (:param?) parameters - route-switch-logic: DOM traversal and sibling matching logic for implementing first-match-wins routing - component-key: Deterministic key generation for forcing component recreation when route parameters change Component naming rationale: Uses kompendium-* prefix instead of stencil-* to clearly indicate these are custom Kompendium components, not official Stencil packages. This avoids confusion and potential namespace collisions while maintaining clear ownership. Architecture: - Hash-based navigation using location.hash and hashchange events - Reactive updates via @State decorators - First-match routing: only the first matching route renders - Dynamic component rendering using Stencil's h() function - Domain-specific modules instead of catch-all "utils" files - Comprehensive test coverage (63 tests) for all routing logic File organization follows domain-driven naming: - route-matching: Route pattern parsing and matching - route-switch-logic: Sibling checking and route precedence - component-key: Stable key generation for component reconciliation Updated imports in component/component.tsx, type/type.tsx, and debug/debug.tsx to use the new module structure. Removed @stencil/router dependency from package.json.
…ation Fix type safety: add null to match variable type The match variable should be typed as MatchResults | null since matchRoute() returns MatchResults | null. This fixes a type safety issue where the variable declaration didn't match the return type. Addresses review feedback from @Copilot
…ation Remove unused RouteConfig interface The RouteConfig interface was exported but never used anywhere in the codebase. Removing it reduces maintenance burden. Addresses review feedback from @Copilot
…ation Remove unused historyType prop The historyType prop was declared but not used in the routing implementation. Currently the router only supports hash-based navigation. Removed the prop from both the component definition and its usage to avoid confusion. Addresses review feedback from @Copilot
…ation Remove unused exact prop The exact prop was declared but never used in the route matching logic. All route matching is already exact by default (regex anchored with ^ and $), so this prop has no effect. Removed it to avoid confusion. Addresses review feedback from @Copilot
…ation Add route caching to avoid redundant regex compilation Route patterns are static, so caching parsed results improves performance by avoiding regex compilation on every route match. Implemented a Map-based cache that stores parsed route patterns. Addresses review feedback from @Copilot
…ation Remove unnecessary currentPath variable The local currentPath variable assignment was unnecessary since this.currentPath can be used directly in the function calls. Removed for cleaner code. Addresses review feedback from @Copilot
…ation Fix test suite name to match module name Updated test suite name from 'router-utils' to 'route-matching' for consistency with the actual module name. Addresses review feedback from @Copilot
…ation Fix misleading test name Updated test name from 'returns false for route element without url property in prototype' to 'returns true for route element regardless of url property presence' to accurately reflect the test expectation. Addresses review feedback from @Copilot
Upgraded Stencil and related dependencies: - @stencil/core: 2.6.0 → 4.38.0 - @stencil/sass: 1.4.1 → 3.2.2 - jest: 27.0.3 → 29.7.0 - jest-cli: 24.9.0 → 29.7.0 (updated to match jest version) - puppeteer: 1.19.0 → 22.15.0 - Removed @types/puppeteer (Puppeteer v10+ provides own types) Configuration changes: - Set preferBuiltins: false in nodeResolve config to allow rollup-plugin-polyfill-node to intercept Node.js built-ins (path, process, etc.) before Rollup treats them as external modules. This enables runtime markdown processing with unified/remark libraries by bundling polyfilled implementations instead of leaving bare imports. - Replaced rollup-plugin-node-polyfills (v0.2.1, 2019) with rollup-plugin-polyfill-node (v0.13.0, 2023) for better modern module support. Code changes: - Fixed event handler type in search.tsx (KeyboardEvent → InputEvent) - Updated TypeScript target to es2018 (required for regex dotAll flag)
7abbec4 to
3a4ba6c
Compare
Use caret prefix for puppeteer version for consistency with other dev dependencies. This allows patch and minor updates while preventing breaking major version changes. Addresses review feedback from @Copilot
3a4ba6c to
bb91737
Compare
… built-in functions
bb91737 to
8f9b8ad
Compare
Upgraded TypeDoc from v0.17.8 to v0.23.28 to enable documentation generation for projects using Stencil v4 and modern TypeScript configurations. Primary motivation: TypeDoc 0.17.8 (July 2020) couldn't properly resolve module paths in Stencil v4 projects using TypeScript 4.7+, causing missing documentation in modern code bases. New capabilities: - Proper module path resolution with TypeScript 4.7+ configurations - @inheritdoc tag support: properties and methods can now inherit documentation from implemented interfaces TypeDoc 0.23 API migration: - Adapted to new comment structure (summary instead of shortText/text) - Updated tag handling (blockTags with content arrays) - Changed method detection (reflection type signatures) - Fixed enum value extraction (type.value for literals) - Added AST parsing workaround for decorator information (TypeDoc 0.23 removed the decorators property) - Improved error handling for file operations in generator cache - Fixed incorrect capitalization of `@inheritDoc` in fixtures BREAKING CHANGE: Kompendium now requires consumers to use TypeScript 4.6.x or higher (previously >= 3.8.3). Projects using TypeScript 3.x or 4.0-4.5 must upgrade their TypeScript version to generate docs.
8f9b8ad to
fd74ec1
Compare
When using a typeroot that includes `src/components.d.ts`, the generated documentation included unwanted types. Implemented filtering to exclude: - Third-party types (node_modules) - Examples and test files - Stencil auto-generated types (components.d.ts, HTML*Element, *CustomEvent) - Types in Components namespace (already documented elsewhere) Uses three-layer strategy: source path filtering, namespace detection, and name pattern matching. Includes cross-platform path support and integration tests with representative fixtures.
fd74ec1 to
72e95ee
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Test-PRs in consumer repos:
lime-crm-building-blocks: https://github.com/Lundalogik/lime-crm-building-blocks/pull/1238
There is one type that is documented in the docs for
latest, which is not documented in the docs for the PR above, but it is theCommandRegistrydefined here, and that shouldn't be included in the docs:lime-elements: Update kompendium Lundalogik/lime-elements#3708
The docs for
latestincorrectly include 6 types that shouldn't be documented:Bird,Food,Habitat,Nest(all fromtable/examples/birds.ts), andShowcaseItem(fromexamples/whats-new/)JSONSchema7(third-party type fromjson-schemapackage)The PR correctly excludes these types and adds
EventEmitter(Stencil core type that's re-exported with@public).However, the PR is missing 6 legitimate types that exist in the source code:
EditorMenuTypes,EditorTextLink,LevelMapping,MouseButtons,ProseMirrorAdapterElementWithFocus(all fromtext-editor/prosemirror-adapter/menu/types.ts), andTextEditorPlugin(fromsrc/components/text-editor/types.ts).lime-elements (with lime-elements upgraded to Stencil 4): Update stencil to v4 Lundalogik/lime-elements#3655
Same 6 types correctly excluded as above (5 example types +
JSONSchema7), and correctly addsEventEmitter. This PR correctly includes all legitimate types - the 5 text-editor types that were missing in #3708 are present here. ✅ (TextEditorPluginis no longer "missing", as it has been removed in this PR.)Testing
This branch of Kompendium has been packed and installed in: