Add knip linter and fix all unused dependencies and exports #234
Conversation
…nd IBindingHandlerObject as central types in @tko/utils
📝 WalkthroughWalkthroughAdds a knip-based dependency/import CI workflow and Makefile targets, removes many transitive Changes
Sequence Diagram(s)sequenceDiagram
participant PR as Pull Request
participant GH as GitHub Actions
participant Runner as CI Runner (Node/NPM)
participant Make as Makefile
participant Knip as knip
participant Lerna as lerna
PR->>GH: trigger workflow (pull_request or workflow_dispatch)
GH->>Runner: start runner, checkout repo
Runner->>Runner: setup Node.js 24 and cache npm
Runner->>Make: run `make` (build)
Runner->>Make: run `make knip`
Make->>Knip: invoke knip via npx to analyze imports/exports
Knip-->>Make: report dependency/import issues
Runner->>Make: run `make lerna-check`
Make->>Lerna: run lerna across packages to validate packages
Lerna-->>Make: report results
Make-->>Runner: exit status
Runner-->>GH: post workflow status
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 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
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ 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). (1)
🔇 Additional comments (1)
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 |
![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 @knip.json:
- Line 11: The ignoreDependencies pattern "karma-[.]*" is wrong and won't match
plugins like "karma-chrome-launcher"; update the value in the
"ignoreDependencies" array by replacing the token "karma-[.]*" with a regex that
matches any karma plugin prefix such as "karma-.+" so that entries like
"karma-mocha" and "karma-chrome-launcher" are ignored as intended.
In @packages/utils.jsx/src/JsxObserver.ts:
- Line 467: Remove the stray comment line that reads "//Duplicated export
default JsxObserver" which sits next to the redundant default export for the
JsxObserver symbol; simply delete that comment line (and any adjacent redundant
default export if present) so only the existing named export of JsxObserver
remains and no cluttering comment is left in the file.
🧹 Nitpick comments (9)
packages/computed/src/computed.ts (1)
70-70: Consider either removing entirely or keeping the export.Commenting out code with a TODO is generally discouraged. The type
MaybeComputedappears nowhere else in the codebase, confirming it is truly unused. Either remove it entirely, or—if this utility type provides value for library consumers (accepting plain values or computed values)—restore the export and remove the TODO comment.packages/observable/src/observableArray.ts (1)
15-15: Prefer removing unused code over commenting it out.Commented-out code adds noise and tends to stay indefinitely. If
MaybeObservableArrayis truly unused, remove it entirely. If it might be needed by external consumers or for future use, consider:
- Keeping the export and adding a Knip ignore directive if it's part of the public API surface
- Creating a tracking issue if restoration is planned
Option 1: Remove entirely
-//TODO Currently unused --> export type MaybeObservableArray<T = any> = T[] | ObservableArray<T>Option 2: Keep as public API with Knip ignore
-//TODO Currently unused --> export type MaybeObservableArray<T = any> = T[] | ObservableArray<T> +// knip-ignore-next - Public API for consumers +export type MaybeObservableArray<T = any> = T[] | ObservableArray<T>packages/utils.component/package.json (1)
16-22: Consider updating stale peerDependencies versions.The
peerDependenciesreference alpha versions (^4.0.0-alpha8.0,^4.0.0-alpha9.0) while the regular dependencies are at^4.0.0-beta1.3. This version gap may cause resolution warnings or compatibility confusion for consumers.If not addressed in this PR, consider tracking this for a follow-up update.
packages/utils/src/interfaces.ts (1)
42-45: Potential circular type reference between IKnockoutUtilsInstance and Options.
IKnockoutUtilsInstancereferencesOptions, andOptions(in options.ts) referencesIKnockoutUtilsInstance. While this works with type-only imports, consider documenting this bidirectional relationship or using a more minimal options type here to reduce coupling.packages/provider/src/BindingHandlerObject.ts (1)
26-29: Consider adding type annotation togetparameter.The
nameOrDottedparameter lacks a type annotation. For consistency with the interface and the typedsetmethod, consider adding: string.Suggested fix
- get(nameOrDotted) { + get(nameOrDotted: string) { const [name] = nameOrDotted.split('.') return this[name] }packages/provider/src/Provider.ts (1)
58-59: Address or remove the TODO comment.The TODO comment flags
postProcessas a potentially unused hook. Consider either removing the commented-out code if it's confirmed unused, or implementing the hook if it's needed for extensibility.Would you like me to help search the codebase to confirm whether
postProcessis used anywhere, or open an issue to track this cleanup?packages/builder/src/Builder.ts (2)
109-128: Minor redundancy:addOrRemoveItemis declared twice.
addOrRemoveItemis already part ofIArrayAndObjectUtils(seepackages/utils/src/interfaces.tslines 23-39), so the explicit declaration on line 110 is redundant. Consider removing it for clarity.♻️ Suggested fix
export type Utils = IArrayAndObjectUtils & { - addOrRemoveItem: typeof addOrRemoveItem cloneNodes: typeof cloneNodes compareArrays: typeof compareArrays
163-171: Clarify the type-merging comment.The comment on line 171 is unclear. Consider rephrasing for better readability.
♻️ Suggested fix
export type KnockoutInstance = IKnockoutUtilsInstance & { // --- Utilities --- cleanNode: typeof cleanNode dependencyDetection: typeof dependencyDetection computedContext: typeof dependencyDetection filters: typeof options.filters ignoreDependencies: typeof dependencyDetection.ignore memoization: typeof memoization - //Type merging from IKnockoutInstance options: typeof options + // Note: `options` property is inherited from IKnockoutUtilsInstance removeNode: typeof removeNode.github/workflows/dependency-cycle.yml (1)
3-7: Consider addingpushtrigger for the main branch.The workflow only runs on
pull_requestandworkflow_dispatch. Consider also triggering on pushes to the main branch to catch any issues that slip through or are introduced via direct commits.Suggested enhancement
on: # Runs for every pull-requests created pull_request: + push: + branches: [main] # Allows you to run this workflow manually from the Actions tab workflow_dispatch:
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (51)
.github/workflows/dependency-cycle.ymlMakefilebuilds/knockout/package.jsonbuilds/reference/package.jsonknip.jsonpackage.jsonpackages/bind/package.jsonpackages/bind/src/applyBindings.tspackages/bind/src/bindingContext.tspackages/binding.component/package.jsonpackages/binding.core/package.jsonpackages/binding.foreach/package.jsonpackages/binding.if/package.jsonpackages/binding.template/package.jsonpackages/binding.template/spec/nativeTemplateEngineBehaviors.tspackages/binding.template/src/nativeTemplateEngine.tspackages/builder/package.jsonpackages/builder/src/Builder.tspackages/computed/package.jsonpackages/computed/src/computed.tspackages/computed/src/proxy.tspackages/filter.punches/package.jsonpackages/lifecycle/package.jsonpackages/observable/package.jsonpackages/observable/src/extenders.tspackages/observable/src/observableArray.tspackages/provider.attr/package.jsonpackages/provider.bindingstring/package.jsonpackages/provider.component/package.jsonpackages/provider.component/spec/customElementBehaviors.tspackages/provider.databind/package.jsonpackages/provider.multi/package.jsonpackages/provider.mustache/package.jsonpackages/provider.mustache/src/mustacheParser.tspackages/provider.native/package.jsonpackages/provider.virtual/package.jsonpackages/provider/package.jsonpackages/provider/src/BindingHandlerObject.tspackages/provider/src/Provider.tspackages/utils.component/package.jsonpackages/utils.functionrewrite/package.jsonpackages/utils.jsx/package.jsonpackages/utils.jsx/src/JsxObserver.tspackages/utils.jsx/src/index.tspackages/utils.parser/package.jsonpackages/utils/package.jsonpackages/utils/src/index.tspackages/utils/src/interfaces.tspackages/utils/src/options.tstsconfig.dts.jsontsconfig.json
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-10T12:14:25.781Z
Learnt from: phillipc
Repo: knockout/tko PR: 233
File: COVERAGE.md:4-6
Timestamp: 2026-01-10T12:14:25.781Z
Learning: In the knockout/tko repository, COVERAGE.md is an auto-generated file (by the test coverage tool) and should be excluded from markdown linting to avoid flagging formatting issues that cannot be manually addressed.
Applied to files:
knip.json
🧬 Code graph analysis (6)
packages/provider/src/BindingHandlerObject.ts (1)
packages/utils/src/index.ts (1)
IBindingHandlerObject(33-33)
packages/utils/src/options.ts (2)
packages/utils/src/index.ts (2)
IKnockoutUtilsInstance(36-36)IProvider(34-34)packages/utils/src/interfaces.ts (2)
IKnockoutUtilsInstance(42-45)IProvider(55-107)
packages/bind/src/bindingContext.ts (2)
packages/utils/src/index.ts (1)
IKnockoutUtilsInstance(36-36)packages/utils/src/interfaces.ts (1)
IKnockoutUtilsInstance(42-45)
packages/utils/src/interfaces.ts (5)
packages/utils/src/index.ts (4)
IBindingHandlerObject(33-33)IArrayAndObjectUtils(37-37)IKnockoutUtilsInstance(36-36)IProvider(34-34)packages/utils/src/array.ts (11)
addOrRemoveItem(80-91)arrayFilter(58-67)arrayFirst(26-32)arrayForEach(9-20)arrayGetDistinctValues(50-56)arrayIndexOf(22-24)arrayMap(34-39)arrayPushAll(69-78)arrayRemoveItem(41-48)compareArrays(149-165)range(98-106)packages/utils/src/symbol.ts (1)
createSymbolOrString(7-9)packages/utils/src/object.ts (2)
objectForEach(32-38)objectMap(40-54)packages/utils/src/options.ts (1)
Options(14-133)
packages/bind/src/applyBindings.ts (2)
packages/utils/src/index.ts (1)
IProvider(34-34)packages/utils/src/interfaces.ts (1)
IProvider(55-107)
packages/builder/src/Builder.ts (1)
packages/utils/src/interfaces.ts (2)
IArrayAndObjectUtils(24-40)IKnockoutUtilsInstance(42-45)
⏰ 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). (1)
- GitHub Check: testheadless (24, test-headless-ff)
🔇 Additional comments (65)
packages/binding.template/src/nativeTemplateEngine.ts (1)
2-2: LGTM!Correctly using inline
typemodifier forTemplateOptionssince it's only used as a type annotation. This aligns with theverbatimModuleSyntaxrequirement and improves clarity about which imports are types versus runtime values.packages/provider.mustache/src/mustacheParser.ts (1)
72-92: LGTM! Proper reduction of public API surface.Removing exports from
innerParseandparseOuterMatchis appropriate since both are internal implementation details only consumed within this module. The public API is correctly limited toparseInterpolation.packages/observable/package.json (1)
33-35: LGTM!The removal of
tslibas part of the Knip cleanup is appropriate. Modern TypeScript configurations typically inline helpers or target ES versions that don't require the runtime library.packages/provider.attr/package.json (1)
11-13: LGTM!Consistent
tslibremoval aligned with the repository-wide dependency cleanup.tsconfig.dts.json (1)
3-8: LGTM!Setting
verbatimModuleSyntaxtonullin the declaration-only config is necessary when usingoutFilefor bundled.d.tsgeneration. TheverbatimModuleSyntaxoption conflicts with module concatenation required byoutFile.packages/utils.jsx/package.json (1)
27-34: LGTM!The
tslibremoval is consistent with the repository-wide cleanup. All required@tko/*dependencies are retained.packages/utils.parser/package.json (1)
11-14: LGTM!The removal of
tslibas part of the Knip cleanup is appropriate. The remaining dependencies (@tko/observableand@tko/utils) are retained correctly.Ensure that the TypeScript compilation settings (e.g.,
verbatimModuleSyntax) are configured such thattslibhelpers are no longer emitted or required at runtime.packages/utils/package.json (1)
30-30: LGTM!Having an empty
dependenciesobject is valid and appropriate for a foundational utility package. This keeps@tko/utilslightweight with no external runtime dependencies, which is ideal for a package that many others depend on.packages/lifecycle/package.json (1)
4-7: LGTM!The
tslibremoval is consistent with the monorepo-wide cleanup. The remaining dependencies (@tko/computedand@tko/utils) are appropriate for the lifecycle package functionality.builds/knockout/package.json (1)
48-64: LGTM!The
tslibremoval is consistent with the monorepo-wide Knip cleanup. The build package correctly retains all necessary@tko/*dependencies for the knockout distribution.packages/binding.component/package.json (1)
7-15: Verify compatibility of@tko/providerto@tko/provider.nativeswitch.The dependency updates appear intentional per the PR scope, but require confirmation that:
- No remaining imports of
@tko/providerexist in this package that would break with the switch to@tko/provider.native@tko/provider.nativeis a compatible replacement and exports the same public API- Consumers of this package won't experience breaking changes from this switch
Check the binding.component source code for any
@tko/providerimports and verify API compatibility.packages/provider.component/package.json (1)
11-18: LGTM!The dependency list is clean after the
tslibremoval. Version constraints are consistent with the monorepo convention.packages/binding.template/package.json (1)
25-30: LGTM!Clean removal of
tslibdependency with consistent versioning across@tko/*packages.builds/reference/package.json (1)
17-35: LGTM!The
tslibremoval is consistent with the broader cleanup effort across the monorepo.packages/filter.punches/package.json (1)
25-27: LGTM!Clean
tslibremoval leaving only the required@tko/observabledependency.packages/computed/package.json (1)
10-13: LGTM - tslib removal aligns with dependency cleanup.Removing the
tslibruntime dependency is appropriate when using modern TypeScript compilation settings that inline helpers.packages/binding.core/package.json (1)
25-30: LGTM - Consistent dependency cleanup.The
tslibremoval is consistent with the broader dependency hygiene effort across the monorepo. All@tko/*dependencies remain aligned at^4.0.0-beta1.3.packages/utils.component/package.json (1)
11-15: LGTM - tslib removal is appropriate.The dependency cleanup aligns with the PR objectives.
packages/provider.mustache/package.json (1)
25-29: Removed dependencies are not imported in the source files.Verification confirms that
@tko/binding.template,@tko/utils, andtslibare not imported anywhere inpackages/provider.mustache/src/. The removal is safe.tsconfig.json (2)
10-10: LGTM! Aligns with tslib removal across packages.Setting
importHelpers: falsecauses TypeScript to inline helper functions rather than importing them fromtslib. This is consistent with the removal oftslibas a dependency across the monorepo packages.
26-26: Setting enforces explicit type-only imports as described.The
verbatimModuleSyntaxoption in tsconfig.json is a valid TypeScript 4.9+ setting that enforces explicitimport typesyntax for type-only imports. This prevents accidental runtime imports of types and supports proper tree-shaking.Note: TypeScript is not available in the sandbox environment, so the suggested compilation verification (
npx tsc --noEmit) could not be completed. This verification should be performed locally or in the project's CI pipeline to confirm all existing imports are compatible with the stricter enforcement.packages/binding.if/package.json (1)
25-29: LGTM! Dependency cleanup aligns with Knip findings.The removal of
@tko/computedandtslib(as indicated by the AI summary) is consistent with the PR's objective to fix unused dependency warnings reported by Knip.packages/binding.template/spec/nativeTemplateEngineBehaviors.ts (1)
4-4: LGTM! Correct use of type-only import.
ObservableArrayis used only as a type annotation (at line 147), whileobservableandobservableArrayare used as runtime values. The inlinetypemodifier correctly distinguishes this, satisfying theverbatimModuleSyntaxrequirement.packages/provider.component/spec/customElementBehaviors.ts (1)
3-3: LGTM! Correct use of type-only import.
Observableis used only as a type (in the interface extension at line 276:interface myObs extends Observable), whileobservable,isWritableObservable, andisObservableare used as runtime values. The inlinetypemodifier is appropriate here.package.json (1)
22-22: LGTM! New dev dependencies for tooling support.Adding
@types/nodefor Node.js type definitions andknipfor unused code detection aligns with the PR objectives. Both package versions are valid and have no security vulnerabilities.packages/binding.foreach/package.json (1)
25-29: LGTM!The dependency cleanup removing
@tko/providerandtslibaligns with the Knip findings. The remaining dependencies (@tko/bind,@tko/observable,@tko/utils) represent the actual runtime requirements for the foreach binding.packages/utils.functionrewrite/package.json (1)
11-11: LGTM!Removing the
tslibruntime dependency is appropriate if the package doesn't use TypeScript helpers that require it, or if helpers are now inlined. This aligns with the broader PR effort to clean up unused dependencies identified by Knip.packages/computed/src/proxy.ts (2)
91-102: LGTM — internal helper utilities are cleanly structured.The
@internalJSDoc annotations appropriately signal that these are not part of the public API. The implementations are straightforward:
getObservableretrieves the mirror observablepeekdelegates to the observable's peek methodisProxiedchecks for the proxy symbolSince these are internal utilities, they reasonably assume valid inputs from internal callers.
104-104: Good pattern for exposing internal utilities.Attaching the helpers to the
proxyfunction object provides a convenient namespace-style API (proxy.getObservable(...)) while the named exports enable tree-shaking for consumers who only need specific utilities.packages/observable/src/extenders.ts (4)
39-44: Visibility reduction:notifyis no longer directly exportable.The function is now internal but remains accessible via the
extendersobject. This is appropriate API hygiene — consumers should use the extender system rather than importing these functions directly.
46-53: Visibility reduction:deferredis no longer directly exportable.Same pattern as
notify— the function remains accessible through theextendersobject while reducing the public API surface.
55-73: Visibility reduction:rateLimitis no longer directly exportable.Consistent with the other extender functions. The implementation logic remains unchanged.
83-83: No breaking changes from visibility modifications.Direct imports of
notify,deferred, andrateLimitare not used in the codebase. All internal consumers import these functions via theextendersobject, which continues to expose them unchanged. The visibility change is backward compatible.packages/provider.databind/package.json (1)
11-14: Dependency changes are justified.The addition of
@tko/bindis confirmed—it's imported inpackages/provider.databind/src/DataBindProvider.ts(import type { BindingContext } from '@tko/bind'). The removal oftslibis also correct, as it was not being used in the source code. These changes align with the cleanup objectives.packages/utils/src/interfaces.ts (4)
1-17: LGTM on imports structure.Clean separation of runtime imports for
typeofusage and type-only import forOptions. This correctly avoids runtime circular dependencies while establishing type contracts.
19-22: Interface aligns with BindingHandlerObject implementation.The
IBindingHandlerObjectinterface correctly captures thesetandgetmethod signatures. Verified againstpackages/provider/src/BindingHandlerObject.tswhich implements this interface.
24-40: IArrayAndObjectUtils correctly aggregates utility function types.Using
typeofto reference the actual function implementations ensures the interface stays synchronized with the implementations in./array,./object, and./symbol.
49-107: IProvider interface is well-documented with proper implementation coverage.The interface provides comprehensive JSDoc documentation for each member. Verification confirms the
Providerclass properly implements all required members:
FOR_NODE_TYPES: Implemented as a getter in the abstract class, overridden in all subclassessetGlobals(),nodeHasBindings(),getBindingAccessors(),preprocessNode(),makeBindingAccessors(): All implementedbindingHandlers,instance,preemptive: All present with correct behaviorNote on the type narrowing suggestion: While
FOR_NODE_TYPESusesnumber[], narrowing it to(1 | 3 | 8)[]is not practical. TheMultiProviderclass stores node types dynamically in a variable, and various providers legitimately use different combinations of values. The current type width appropriately supports the abstract provider pattern and allows flexibility for extensions.The
instanceproperty correctly implements legacy compatibility through a getter/setter that wraps the assigned provider in aLegacyProviderwhen needed.packages/provider/src/BindingHandlerObject.ts (1)
2-4: Clean implementation of the new interface.The type-only import and
implements IBindingHandlerObjectclause correctly enforce the type contract without affecting runtime behavior.packages/utils/src/options.ts (3)
1-1: Type-only imports correctly break cyclic dependencies.Importing interfaces instead of concrete types from
./interfaceseliminates the cycle that would occur if importing fromProvideror other packages.
19-19: Type change to interface preserves API while enabling decoupling.Changing from a concrete type to
IKnockoutUtilsInstancemaintains compatibility while allowing the options module to be independent of the full knockout instance implementation.
40-41: This review comment is based on an incorrect assumption about TypeScript configuration.
strictPropertyInitializationis explicitly set tofalseintsconfig.json, so TypeScript will not flag this uninitialized property. The property is intentionally left uninitialized and is expected to be set after construction.Likely an incorrect or invalid review comment.
packages/utils/src/index.ts (1)
32-38: Type re-exports correctly establish the public API surface.Using
export type { ... }aligns with the PR's enablement ofverbatimModuleSyntaxand ensures these interfaces are available to consuming packages like@tko/providerand@tko/builder.packages/utils.jsx/src/index.ts (1)
3-3: Breaking change: JsxObserver is now a named export.This changes the import pattern from:
import JsxObserver from '@tko/utils.jsx'to:
import { JsxObserver } from '@tko/utils.jsx'All internal consumers in the monorepo have been updated to use the named import pattern.
packages/provider/src/Provider.ts (3)
9-10: LGTM!Good refactor to import
BindingAccessorsandIProviderfrom the centralized@tko/utilspackage, aligning with the PR's goal to consolidate type definitions.
19-19: LGTM!Implementing
IProviderinterface ensures theProviderclass adheres to the expected contract and improves type safety across the codebase.
93-93: LGTM!The explicit type annotation on
makeBindingAccessorsaligns with theIProviderinterface contract.packages/builder/src/Builder.ts (1)
40-42: LGTM!Good use of type-only imports for
IKnockoutUtilsInstanceandIArrayAndObjectUtils, aligning withverbatimModuleSyntaxrequirements.packages/bind/src/applyBindings.ts (3)
19-19: LGTM!Correct use of type-only import syntax for
Computed, consistent with TypeScript'sverbatimModuleSyntaxsetting.
29-29: LGTM!Switching from concrete
Providerclass toIProviderinterface follows the dependency inversion principle. This decouples the binding logic from the specific provider implementation.
60-67: LGTM!The return type and parameter type updates to
IProviderare consistent with the interface-based refactor.packages/bind/src/bindingContext.ts (3)
1-1: LGTM!Clean import combining value imports with type-only import for
IKnockoutUtilsInstance.
11-11: LGTM!Correct use of type-only import for
BindingContextExtendCallback.
24-26: The type narrowing ofkotoIKnockoutUtilsInstancedoesn't break any existing usages in the codebase.The
koproperty type is indeed narrowed from the fullKnockoutInstancetoIKnockoutUtilsInstance, which only exposesutilsandoptions. At runtime, the full instance is still assigned (line 82), but TypeScript will only allow access to the narrower interface. However, a comprehensive search of the codebase reveals no binding code or templates that accesskoproperties (e.g.,ko.observable,ko.computed) via$context.koorcontext.ko, so this type narrowing does not introduce any type errors in practice.packages/provider.bindingstring/package.json (1)
11-14: LGTM on removingtslib.With
importHelpers: falsein the roottsconfig.json, TypeScript does not emit helper imports from tslib, making it unnecessary as a runtime dependency. Removing it frompackages/provider.bindingstring/package.jsonis correct.Makefile (2)
54-56: LGTM!The
kniptarget is well-configured with--no-config-hintsto suppress noisy suggestions in CI output.
69-71: LGTM!The
lerna-checktarget provides a lightweight validation that lerna can resolve all package dependencies, which effectively catches circular dependency issues and broken package references.knip.json (1)
1-20: LGTM overall!The knip configuration is well-structured for a monorepo setup. The settings appropriately:
- Ignore type/interface exports that are used within the same file
- Exclude common dev/test dependencies from unused dependency warnings
- Configure workspace-specific entry points and project patterns
packages/bind/package.json (1)
6-11: The@tko/providerdependency removal frompackages/bind/package.jsonis safe. No source files in this package import from@tko/provider.packages/provider.multi/package.json (1)
11-14: The@tko/binddependency is actually imported inpackages/provider.multi/src/MultiProvider.ts(line 5:import type { BindingContext } from '@tko/bind'), so this is a valid and necessary dependency addition..github/workflows/dependency-cycle.yml (1)
17-21: No action required. The workflow configuration is correct.
actions/setup-node@v6is the current latest version (v6.1.0 as of December 2025), and Node.js 24 is available on GitHub-hosted runners and will become the default in March 2026. Both versions are appropriate for this workflow.Likely an incorrect or invalid review comment.
packages/provider.virtual/package.json (1)
12-16: Dependency updates look correct.The addition of
@tko/bindand removal oftslibaligns with the Knip cleanup objectives. Version specifications are consistent with other packages in the monorepo.packages/provider.native/package.json (1)
11-15: Dependency restructuring looks appropriate.The package now explicitly declares its direct dependencies (
@tko/bind,@tko/observable,@tko/provider) rather than relying on transitive dependencies throughtslib. Version specifications are consistent at^4.0.0-beta1.3.packages/provider/package.json (1)
12-16: Dependency updates align with the refactoring goals.The base provider package now explicitly declares
@tko/bind,@tko/observable, and@tko/utilsas direct dependencies, removing thetslibtransitive dependency. Version specifications are consistent across the monorepo.packages/builder/package.json (1)
11-20: Dependency updates look correct for the builder package.The addition of
@tko/providerand@tko/binding.templatewhile removing unused dependencies aligns with the Knip cleanup objectives.Note: Most
@tko/*dependencies use^4.0.0-beta1.3, while@tko/utils.parseruses^4.0.0-beta1.6. This is correct—@tko/utils.parseris published at 4.0.0-beta1.6 in the monorepo, making this version pinning accurate.
phillipc
changed the title
brianmhunt
left a comment
brianmhunt
left a comment
There was a problem hiding this comment.
The Knip part looks great, but I don't understand the point or benefit of the I* interfaces, and it will almost certainly break existing things. Is there a reason to add it? If there is a benefit let's avoid the I* convention and just go with the type naming styles we already have.
| } | ||
| postProcess(/* node */) {} | ||
|
|
||
| //TODO unused hook? postProcess(/* node */) {} |
There was a problem hiding this comment.
I think leave postProcess there, it is meant to be overloaded, and the no-op serves for typing Provider generally. Unless it's never used, in which case we can probably take it out (but I feel like it was used, so we should be sure)
There was a problem hiding this comment.
Okay, I will remove it. This hook is never called.
There was a problem hiding this comment.
No call, no override, only definited as noop:
| import type { Observable, ObservableFunctions } from './observable' | ||
|
|
||
| export type MaybeObservableArray<T = any> = T[] | ObservableArray<T> | ||
| //TODO Currently unused --> export type MaybeObservableArray<T = any> = T[] | ObservableArray<T> |
There was a problem hiding this comment.
this is definitely used as an ambient declaration like KnockoutObservable, but perhaps not this version here.
| import type { Observable, ObservableFunctions } from './observable' | ||
|
|
||
| export type MaybeObservableArray<T = any> = T[] | ObservableArray<T> | ||
| //TODO Currently unused --> export type MaybeObservableArray<T = any> = T[] | ObservableArray<T> |
There was a problem hiding this comment.
this is definitely used as an ambient declaration like KnockoutObservable, but perhaps not this version here.
Which is to say, I don't know if we can delete it, but lets not since it seems way out of scope for Knip
There was a problem hiding this comment.
Good to discuss. That's why I only commented on it. I can change it to @lintignore
Currently, Knip detected that this type def is unused
There was a problem hiding this comment.
We have ported this type from the official knockout.d.ts to tko, but never used it.
| return proxy | ||
| } | ||
|
|
||
| /** @internal */ |
There was a problem hiding this comment.
I don't know what this is or why we'd add it, so lets skip it unless there's a clear need.
If something is internal it should be private or #private. Also seems out of scope for Knip
There was a problem hiding this comment.
The reason for this is the export. In my opinion, the export only happens for testability in the same package. @internal is typescript sugar for such cases. https://www.typescriptlang.org/tsconfig/#stripInternal
Knip recognize this, too. And understand in this way that this is not an unnecessary export.
| * Providers are responsible for detecting bindings on DOM nodes and | ||
| * producing binding-accessor objects that the binding system consumes. | ||
| */ | ||
| export interface IProvider { |
There was a problem hiding this comment.
the authority and single source of truth is Provider. We dont want a redundant arbitrary interface that duplicates the class definition that we already have. No IAnything.
There was a problem hiding this comment.
I'm with you. I proceeded this way to break the cyclical packages dependency. Utils <-> Provider and Bind. Any other ideas?
There was a problem hiding this comment.
Probably an abstract base class? ProviderBase? (I wouldn't want to change the package structure.)
packages/utils/src/interfaces.ts
Outdated
| get(nameOrDotted: string): any | ||
| } | ||
|
|
||
| export interface IArrayAndObjectUtils { |
There was a problem hiding this comment.
this is a hard-coded subset of:
import * as array from 'array'
typeof arraybut its not clear what benefit it adds, or what it has to do with Knip.
There was a problem hiding this comment.
It also breaks the cyclic reference between Builder.ts and Options.ts...
There was a problem hiding this comment.
We need a type structure like this part for defining the return type of builder.create in builder.ts. This is a subset of some utils classes.
packages/utils/src/options.ts
Outdated
|
|
||
| // Reference to the own knockout instance | ||
| knockoutInstance: KnockoutInstance | null = null | ||
| knockoutInstance: IKnockoutUtilsInstance | null = null |
There was a problem hiding this comment.
I feel like this is both misleading and incorrect and will break a lot. Let's take out all the I* things, it seems unrelated/unnecessary.
There was a problem hiding this comment.
Done, changed to KnockoutUtils
| import type { IBindingHandlerObject } from '@tko/utils' | ||
|
|
||
| export default class BindingHandlerObject { | ||
| export default class BindingHandlerObject implements IBindingHandlerObject { |
There was a problem hiding this comment.
we're never going to have anything else that implements this, and now we have two sources of truth, making this fragile, redundant, and confusing.
There was a problem hiding this comment.
I've removed IBindingHandlerObject for now and replaced it with "any" in IProvider. This way, we're only dealing with the minimal problem with IProvider. Any idea?
Here again is the initial situation:
If this is corrected – then type cycle. In this case, everything still works because there are type imports. However, that's not correct.
packages/builder/src/Builder.ts
Outdated
| } | ||
|
|
||
| export type KnockoutInstance = { | ||
| export type KnockoutInstance = IKnockoutUtilsInstance & { |
There was a problem hiding this comment.
I'm unclear why we would do this. Also we don't want a novel naming convention I* that's inconsistent with everything else in the project. We can just call it KnockoutUtils. It can also be a class.
There was a problem hiding this comment.
I* is removed. We need another solution for the IProvider.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/provider.mustache/src/AttributeMustacheProvider.ts:
- Around line 81-84: In getPossibleDirectBinding (AttributeMustacheProvider),
you’re computing bindingName from ATTRIBUTES_BINDING_MAP but then incorrectly
passing attrName to this.bindingHandlers.get; change the lookup to use
bindingName instead (i.e., return bindingName &&
this.bindingHandlers.get(bindingName)) so mapped names like 'class' → 'css'
resolve the correct handler; ensure no other logic relies on the original
attrName being passed.
🧹 Nitpick comments (2)
packages/builder/src/Builder.ts (1)
162-170: Clean up the comment artifact on line 170.The comment
//Type merging from IKnockoutInstance options: typeof optionsappears to be an artifact from a previous implementation attempt. Consider removing it or reformatting it as a proper explanatory comment.Since
KnockoutUtilsis defined as{ utils: ArrayAndObjectUtils; options: Options }, theoptionsproperty is correctly provided via the intersection type - the comment is redundant.Suggested fix
export type KnockoutInstance = KnockoutUtils & { // --- Utilities --- cleanNode: typeof cleanNode dependencyDetection: typeof dependencyDetection computedContext: typeof dependencyDetection filters: typeof options.filters ignoreDependencies: typeof dependencyDetection.ignore memoization: typeof memoization - //Type merging from IKnockoutInstance options: typeof options + // Note: `options` property is provided via KnockoutUtils intersection removeNode: typeof removeNodepackages/utils/src/interfaces.ts (1)
39-39: Consider stricter typing for BindingAccessors.Using
Functionis a loose type. Consider a more specific signature if the accessor pattern is consistent:export type BindingAccessors = { [name: string]: () => unknown }However, if accessors have varying signatures, the current definition is acceptable.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
.github/workflows/dependency-cycle.ymlknip.jsonpackages/bind/src/bindingContext.tspackages/builder/src/Builder.tspackages/provider.mustache/src/AttributeMustacheProvider.tspackages/provider/src/BindingHandlerObject.tspackages/utils.jsx/src/JsxObserver.tspackages/utils/src/index.tspackages/utils/src/interfaces.tspackages/utils/src/options.ts
💤 Files with no reviewable changes (1)
- packages/utils.jsx/src/JsxObserver.ts
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/workflows/dependency-cycle.yml
- packages/bind/src/bindingContext.ts
- knip.json
- packages/utils/src/index.ts
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2026-01-03T18:06:10.239Z
Learnt from: phillipc
Repo: knockout/tko PR: 233
File: packages/binding.core/src/value.ts:64-72
Timestamp: 2026-01-03T18:06:10.239Z
Learning: In packages/binding.core/src/value.ts, the ieAutoCompleteHackNeeded getter and related propertyChange/focus/blur event handlers for IE10/11 autocomplete workarounds should be kept, as IE10/11 support is intentionally retained even though IE9 support has been removed.
Applied to files:
packages/provider.mustache/src/AttributeMustacheProvider.ts
🧬 Code graph analysis (3)
packages/provider/src/BindingHandlerObject.ts (2)
packages/utils/src/index.ts (1)
IBindingHandlerObject(33-33)packages/utils/src/interfaces.ts (1)
IBindingHandlerObject(41-44)
packages/builder/src/Builder.ts (2)
packages/utils/src/index.ts (3)
ArrayAndObjectUtils(37-37)KnockoutUtils(36-36)memoization(39-39)packages/utils/src/interfaces.ts (2)
ArrayAndObjectUtils(19-35)KnockoutUtils(37-37)
packages/utils/src/options.ts (2)
packages/utils/src/index.ts (2)
KnockoutUtils(36-36)IProvider(34-34)packages/utils/src/interfaces.ts (2)
KnockoutUtils(37-37)IProvider(52-104)
⏰ 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). (1)
- GitHub Check: testheadless (24, test-headless-ff)
🔇 Additional comments (11)
packages/builder/src/Builder.ts (2)
40-43: LGTM! Clean type imports.The addition of
virtualElementsas a value import and the type-only imports forKnockoutUtilsandArrayAndObjectUtilsproperly support the type composition changes below.
109-127: LGTM! Cleaner type composition using intersection.Using
ArrayAndObjectUtils &intersection instead of duplicating all array utility members directly in theUtilstype is a good DRY improvement. The type now derives array/object utilities from the shared definition in@tko/utils.packages/provider/src/BindingHandlerObject.ts (2)
1-4: LGTM! Good interface extraction.The class now properly implements
IBindingHandlerObject, centralizing the contract in@tko/utils. This helps break cyclic dependencies and establishes a clear public API surface.
26-29: LGTM! Explicit parameter typing.Adding the explicit
stringtype to thenameOrDottedparameter aligns with theIBindingHandlerObjectinterface contract.packages/utils/src/options.ts (3)
1-1: LGTM! Type-only imports from local interfaces.Using type-only imports from
./interfacesproperly supports theverbatimModuleSyntaxTypeScript option enabled in this PR.
19-19: LGTM! Narrower type for knockoutInstance.Changing from the full
KnockoutInstancetype toKnockoutUtilshelps break cyclic type dependencies while still providing the essential utilities and options needed at this level.
41-41: No issue.strictPropertyInitializationis explicitly set tofalsein tsconfig.json, which allows the uninitialized property declaration. This is a valid and intentional configuration choice.packages/utils/src/interfaces.ts (4)
1-17: LGTM! Well-organized imports.The imports correctly bring in the necessary functions and types from local modules to construct the centralized type definitions.
19-37: LGTM! Clean utility type definitions.
ArrayAndObjectUtilsproperly captures the signatures of all array/object utility functions usingtypeof, ensuring type safety.KnockoutUtilsprovides a minimal interface for breaking cyclic dependencies.
46-104: LGTM! Well-documented IProvider interface.The interface is comprehensive with good JSDoc documentation explaining each member's purpose. This provides a clear contract for binding providers and helps decouple the provider implementation from consumers.
41-44: Update thevalueparameter type in thesetmethod to reflect actual usage.The type annotation
value?: string | objectis too restrictive. Usage throughout the codebase passes functions and objects with function properties (e.g., binding handler definitions withinitandupdatemethods). The implementation at line 7 stores values viathis[nameOrObject] = value, which allows any type. The parameter type should beanyor a more specific binding handler type to match the actual implementation behavior and real-world usages.
| getPossibleDirectBinding(attrName: string) { | ||
| const bindingName = this.ATTRIBUTES_BINDING_MAP[attrName] | ||
| return bindingName && this.bindingHandlers.get(attrName) | ||
| return bindingName && this.bindingHandlers.get(attrName) //FIXME this.bindingHandlers.get(bindingName) ? | ||
| } |
There was a problem hiding this comment.
Bug: Using attrName instead of bindingName for handler lookup.
The method retrieves bindingName from ATTRIBUTES_BINDING_MAP (e.g., 'class' → 'css') but then passes attrName to bindingHandlers.get(). This means for attributes like class, the code looks up a class handler instead of the intended css handler.
The FIXME comment correctly identifies the issue—this should be fixed rather than deferred.
🐛 Proposed fix
getPossibleDirectBinding(attrName: string) {
const bindingName = this.ATTRIBUTES_BINDING_MAP[attrName]
- return bindingName && this.bindingHandlers.get(attrName) //FIXME this.bindingHandlers.get(bindingName) ?
+ return bindingName && this.bindingHandlers.get(bindingName)
}Note: If this is intentional behavior, please clarify the purpose of ATTRIBUTES_BINDING_MAP since the mapped value is currently unused.
📝 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.
| getPossibleDirectBinding(attrName: string) { | |
| const bindingName = this.ATTRIBUTES_BINDING_MAP[attrName] | |
| return bindingName && this.bindingHandlers.get(attrName) | |
| return bindingName && this.bindingHandlers.get(attrName) //FIXME this.bindingHandlers.get(bindingName) ? | |
| } | |
| getPossibleDirectBinding(attrName: string) { | |
| const bindingName = this.ATTRIBUTES_BINDING_MAP[attrName] | |
| return bindingName && this.bindingHandlers.get(bindingName) | |
| } |
🤖 Prompt for AI Agents
In @packages/provider.mustache/src/AttributeMustacheProvider.ts around lines 81
- 84, In getPossibleDirectBinding (AttributeMustacheProvider), you’re computing
bindingName from ATTRIBUTES_BINDING_MAP but then incorrectly passing attrName to
this.bindingHandlers.get; change the lookup to use bindingName instead (i.e.,
return bindingName && this.bindingHandlers.get(bindingName)) so mapped names
like 'class' → 'css' resolve the correct handler; ensure no other logic relies
on the original attrName being passed.
There was a problem hiding this comment.
@coderabbitai Please create an issue for that. We need to fix it separately and also require a test case.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Create a GitHub issue for the bug in AttributeMustacheProvider.getPossibleDirectBinding
gh issue create \
--title "Bug in AttributeMustacheProvider.getPossibleDirectBinding: using attrName instead of bindingName" \
--body "## Description
In \`packages/provider.mustache/src/AttributeMustacheProvider.ts\`, the \`getPossibleDirectBinding\` method has a bug where it retrieves \`bindingName\` from \`ATTRIBUTES_BINDING_MAP\` but then incorrectly passes \`attrName\` to \`bindingHandlers.get()\`.
This means for attributes like \`class\`, the code looks up a \`class\` handler instead of the intended \`css\` handler (since \`DEFAULT_ATTRIBUTE_BINDING_MAP\` maps \`'class'\` → \`'css'\`).
The method currently has a FIXME comment acknowledging this issue.
## Current Behavior
\`\`\`typescript
getPossibleDirectBinding(attrName: string) {
const bindingName = this.ATTRIBUTES_BINDING_MAP[attrName]
return bindingName && this.bindingHandlers.get(attrName) //FIXME this.bindingHandlers.get(bindingName) ?
}
\`\`\`
The mapped \`bindingName\` value is computed but never used in the lookup.
## Expected Behavior
The method should use \`bindingName\` for the handler lookup so that attribute mappings work correctly.
## Requirements
- Fix the handler lookup to use the correct binding name
- Add test case to verify the fix works correctly
- Ensure attribute mappings like 'class' → 'css' resolve the correct handler
## Context
Reported by: @phillipc
Related PR: #234
Review comment: https://github.com/knockout/tko/pull/234#discussion_r2688341614" \
--assignee phillipc
Length of output: 42
@phillipc, I've created issue #235 to track this bug. The issue includes:
- A detailed description of the bug in
getPossibleDirectBinding - The current vs. expected behavior
- Requirements for the fix (including the need for a test case)
- Backlinks to this PR and review comment
The issue has been assigned to you for follow-up.
🧬 Code Graph Analysis Results
packages/provider.src/index.ts
ProviderParamsInput
packages/provider/src/Provider.ts
export default class Provider implements IProvider {
constructor(params?: ProviderParamsInput | null) {
if (this.constructor === Provider) {
throw new Error('Provider is an abstract base class.')
}
if (!('FOR_NODE_TYPES' in this)) {
// FOR_NODE_TYPES must return a list of integers corresponding to the
// node.nodeType's that the provider handles.
throw new Error('Providers must have FOR_NODE_TYPES property')
}
this.bindingHandlers = params?.bindingHandlers || new BindingHandlerObject()
this.globals = params?.globals || {}
}
get FOR_NODE_TYPES(): number[] {
throw new Error('Providers must override FOR_NODE_TYPES property')
}
setGlobals(globals) {
this.globals = globals
}
get preemptive() {
return false
}
nodeHasBindings(node: Node, context?: BindingContext): boolean {
return false
}
getBindingAccessors(node: Node, context?: BindingContext): BindingAccessors {
return Object.create(null)
}
/**
* Preprocess a given node.
* @param {Node} node
* @returns {[Node]|null}
*/
preprocessNode(node: Node): Node[] | null {
return [node]
}
//TODO unused hook? postProcess(/* node */) {}
bindingHandlers: BindingHandlerObject
globals: any | undefined
_overloadInstance: any | undefined
/** For legacy binding provider assignments to
* ko.bindingProvider.instance = ... */
get instance() {
return this._overloadInstance || this
}
set instance(provider) {
if (!provider || provider === this) {
this._overloadInstance = undefined
} else {
this._overloadInstance = new LegacyProvider(provider, this)
}
}
// Given a function that returns bindings, create and return a new object that contains
// binding value-accessors functions. Each accessor function calls the original function
// so that it always gets the latest value and all dependencies are captured. This is used
// by ko.applyBindingsToNode and getBindingsAndMakeAccessors.
makeAccessorsFromFunction(callback) {
return objectMap(dependencyDetection.ignore(callback), (value, key) => () => callback()[key])
}
// Returns the valueAccessor function for a binding value
makeValueAccessor(value): Function {
return () => value
}
// Given a bindings function or object, create and return a new object that contains
// binding value-accessors functions. This is used by ko.applyBindingsToNode.
makeBindingAccessors(bindings: any, context: any, node: Node): BindingAccessors {
if (typeof bindings === 'function') {
return this.makeAccessorsFromFunction(bindings.bind(null, context, node))
} else {
return objectMap(bindings, this.makeValueAccessor)
}
}
}
packages/provider.src/index.ts
ProviderParamsInput
packages/provider.mustache/src/mustacheParser.ts
export function* parseInterpolation(text: string | null) {
for (const textOrExpr of parseOuterMatch(text?.match(OUTER_EXPRESSION))) {
if (textOrExpr.text) {
yield textOrExpr
}
}
}
packages/binding.core/src/attr.ts
export const attr = {
update: function (element, valueAccessor, _allBindings: AllBindings) {
const value = unwrap(valueAccessor()) || {}
objectForEach(value, function (attrName, attrValue) {
attrValue = unwrap(attrValue)
// Find the namespace of this attribute, if any.
const prefixLen = attrName.indexOf(':')
const namespace = prefixLen > 0 && element.lookupNamespaceURI(attrName.substr(0, prefixLen))
// To cover cases like "attr: { checked:someProp }", we want to remove the attribute entirely
// when someProp is a "no value"-like value (strictly null, false, or undefined)
// (because the absence of the "checked" attr is how to mark an element as not checked, etc.)
const toRemove = attrValue === false || attrValue === null || attrValue === undefined
if (toRemove) {
if (namespace) {
element.removeAttributeNS(namespace, attrName)
} else {
element.removeAttribute(attrName)
}
} else {
attrValue = attrValue.toString()
if (namespace) {
element.setAttributeNS(namespace, attrName, attrValue)
} else {
element.setAttribute(attrName, attrValue)
}
}
// Deliberately being case-sensitive here because XHTML would regard "Name" as a different thing
// entirely, and there's no strong reason to allow for such casing in HTML.
if (attrName === 'name') {
element.name = toRemove ? '' : attrValue
}
})
}
}
packages/provider/src/Provider.ts
export default class Provider implements IProvider {
...
}
🧠 Learnings used
Learnt from: phillipc
Repo: knockout/tko PR: 233
File: packages/binding.core/src/value.ts:64-72
Timestamp: 2026-01-03T18:06:10.239Z
Learning: In packages/binding.core/src/value.ts, the ieAutoCompleteHackNeeded getter and related propertyChange/focus/blur event handlers for IE10/11 autocomplete workarounds should be kept, as IE10/11 support is intentionally retained even though IE9 support has been removed.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/utils/src/index.ts:
- Around line 32-37: Remove the trailing whitespace after the opening brace in
the export type statement so the line "export type { IProvider,
BindingAccessors, KnockoutUtils, ArrayAndObjectUtils } from './interfaces'" has
no extra spaces between "{" and "IProvider"; update the export line (export type
{ ... } from './interfaces') to eliminate the stray spaces to satisfy Prettier
formatting.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/provider/src/BindingHandlerObject.tspackages/utils/src/index.tspackages/utils/src/interfaces.ts
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/provider/src/BindingHandlerObject.ts
- packages/utils/src/interfaces.ts
🧰 Additional context used
🪛 GitHub Actions: Check code formatting with Prettier
packages/utils/src/index.ts
[warning] 1-1: Code style issues found by Prettier. Run 'npx prettier --write' to fix formatting.
⏰ 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). (1)
- GitHub Check: testheadless (24, test-headless-ff)
|
@brianmhunt: Summary, Knip detected some unused exports and missing dependencies at package.json level between the mono-repo-projects. While I add the missing project references, some cyclical references were uncovered. To resolve that I have add some interfaces in utils. Now, I have refactor this PR to one last "IProvider" and "KnockoutUtils". I* notations are not typical typescript naming style, we can change it perhaps to an abstract base class like ProviderBase, but this is only an easy stylistic aspect. You are welcome with a better idea to solve this problem. To put this in perspective, all of this is about the path to well-defined TKO types. |
2 participants
Knip finds and fixes unused dependencies, exports and files in your JavaScript and TypeScript projects.
In this PR we configured Knip and fixed all findings.
Summary by CodeRabbit
Chores
Refactor
Build
✏️ Tip: You can customize this high-level summary in your review settings.