-
Notifications
You must be signed in to change notification settings - Fork 1
fix(core): export component types for TypeScript resolution #592
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
Conversation
✅ Deploy Preview for pine-design-system ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Dependency Review✅ No vulnerabilities or license issues or OpenSSF Scorecard issues found.Scanned FilesNone |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
📝 WalkthroughWalkthroughThe changes introduce proper module exports configuration to the core library. A new Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Comment |
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.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
libs/core/package.json(1 hunks)libs/core/src/index.ts(1 hunks)
🔇 Additional comments (2)
libs/core/package.json (2)
26-30: Loader export entry is correctly configured.The "./loader" export aligns with the Stencil build configuration (
esmLoaderPath: '../loader'in stencil.config.ts). The loader files are build artifacts generated during thestencil buildprocess and will be created at the paths specified. The export entry itself requires no changes.
25-25: No issues found—the wildcard export is correctly configured.The
./components/*export is intentional and verified in the Stencil build configuration. Thedist-custom-elementsoutput target explicitly builds to thecomponents/directory at the package root, which is then published (listed in thefilesarray). This export pattern enables direct component imports like'@pine-ds/core/components/pds-button'for better tree-shaking, and only compiled custom elements are included in the published output—test files, stories, and source remain in src/.
| "exports": { | ||
| ".": { | ||
| "types": "./dist/types/index.d.ts", | ||
| "import": "./dist/esm/index.mjs", | ||
| "require": "./dist/index.cjs.js" | ||
| }, |
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.
🧩 Analysis chain
Main export entry looks correct.
The main export paths align with the existing main, module, and types fields, providing proper conditional exports for TypeScript, ESM, and CJS formats. This should significantly improve type resolution in modern IDEs and bundlers.
Verify that the build output generates files at the specified paths:
🏁 Script executed:
#!/bin/bash
# Check if the main export paths exist after build
paths=(
"libs/core/dist/types/index.d.ts"
"libs/core/dist/esm/index.mjs"
"libs/core/dist/index.cjs.js"
)
echo "Checking main export paths..."
for path in "${paths[@]}"; do
if [ -f "$path" ]; then
echo "✓ $path exists"
else
echo "✗ $path NOT FOUND"
fi
doneLength of output: 154
I need to investigate the actual build output and configuration to understand why these files don't exist.
I need to check the actual dist directory contents and the build configuration to understand why these files don't exist.
Critical issue: Package.json exports reference non-existent build artifacts.
The verification confirms that ALL three export paths specified in libs/core/package.json do not exist:
libs/core/dist/types/index.d.ts✗libs/core/dist/esm/index.mjs✗libs/core/dist/index.cjs.js✗
The package will fail to resolve when consumed because the exports field references files that haven't been generated. The build must be executed first to create these artifacts, or the paths in package.json must be corrected to match the actual build output.
Verify that:
- The build script in
libs/core/package.jsongenerates files at these exact paths - The build has been run before publishing
- Alternative: If the build generates files to different paths, update the exports field accordingly
🏁 Script executed:
#!/bin/bash
# Check libs/core/dist directory structure
echo "=== Checking libs/core/dist directory ==="
if [ -d "libs/core/dist" ]; then
find libs/core/dist -type f | head -20
else
echo "libs/core/dist directory does NOT exist"
fi
echo ""
echo "=== Checking build-related files in libs/core ==="
ls -la libs/core/ | grep -E "(package\.json|tsconfig|build|rollup|webpack|vite)" || echo "No obvious build files found"
echo ""
echo "=== Checking libs/core/package.json build scripts ==="
cat libs/core/package.json | grep -A 10 '"scripts"' || echo "No scripts section found"Length of output: 742
I need to check the Stencil build configuration to understand what output it actually generates.
🏁 Script executed:
#!/bin/bash
# Check for Stencil configuration
echo "=== Looking for Stencil config ==="
find libs/core -maxdepth 2 -name "stencil.config.*" -o -name ".stencilrc*" | head -10
echo ""
echo "=== Checking libs/core directory structure ==="
ls -la libs/core/ | grep -v "^total"
echo ""
echo "=== Checking if there's a src directory with TypeScript ==="
if [ -d "libs/core/src" ]; then
find libs/core/src -type f -name "*.ts" -o -name "*.tsx" | head -10
fi
echo ""
echo "=== Checking package.json main/module/exports more thoroughly ==="
cat libs/core/package.json | grep -A 30 '"main"' | head -50Length of output: 3248
Now I need to check the Stencil configuration to verify it generates the expected output paths.
🏁 Script executed:
#!/bin/bash
# Read stencil.config.ts to understand output configuration
echo "=== Stencil Config ==="
cat libs/core/stencil.config.ts
echo ""
echo "=== Check for existing dist output or build artifacts ==="
# Look for any patterns in git or elsewhere that show the dist structure was working
find libs/core -name "*.d.ts" -type f 2>/dev/null | grep -E "dist|types" | head -10
echo ""
echo "=== Check git history for dist files ==="
git log --name-only --oneline libs/core/dist/ 2>/dev/null | head -20 || echo "No git history for dist/"Length of output: 1499
I need to verify what Stencil actually outputs for the 'dist' output target.
🌐 Web query:
Stencil dist output target generates what files and directory structure
💡 Result:
Typical files and folders produced by the Stencil "dist" (distribution) output target
-
dist/ (public distribution directory)
- dist/index.js (CommonJS entry) and/or dist/index.mjs (ESM entry). [1]
- dist/esm/ (modern ES module builds for differential bundling). [1]
- dist/custom-elements/ or dist/dist-custom-elements/ (optional custom-elements build/bundle if configured). [1][2]
- dist/collection/
- dist/collection/index.js (collection entry used by the loader/bundler).
- dist/collection/collection-manifest.json (component metadata used by consumers). [1][2]
- dist/types/ (.d.ts TypeScript declaration files for components). [1]
- dist//.js (optional single-file UMD/unpkg build when configured). [1]
-
loader/ (separate folder, created when esmLoaderPath or loader is used)
- loader/index.mjs, loader/index.cjs, and supporting runtime files used to lazy-load component chunks (the "esm loader"). [1][3]
-
package.json fields typically set to point at dist outputs
- "main": "dist/index.js"
- "module"/"es2015"/"es2017": "dist/index.mjs" or "dist/esm/index.mjs"
- "types": "dist/types/*.d.ts"
- "collection": "dist/collection/collection-manifest.json"
- "collection:main": "dist/collection/index.js"
- "files": often include ["dist/", "loader/"]. [1][2][3]
Notes:
- Exact files and subfolders depend on which additional output-targets or options you enable (e.g., dist-custom-elements, dist-custom-elements-bundle, esmLoaderPath, buildEs5). See the Stencil docs for details. [1][2]
Sources:
[1] Stencil — Distribution / Output Targets documentation.
[2] Stencil — Output Targets and custom-elements bundle docs.
[3] Example package.json / community posts showing typical dist + loader layout.
I've found a critical issue. Let me verify if Stencil has custom configuration to output .cjs.js files, or if the paths in package.json should be corrected.
🏁 Script executed:
#!/bin/bash
# Search for any custom Stencil configuration that might output .cjs.js
echo "=== Searching for .cjs configuration in libs/core ==="
grep -r "\.cjs" libs/core/ --include="*.ts" --include="*.js" --include="*.json" 2>/dev/null | grep -v node_modules | head -20
echo ""
echo "=== Check if there are any build scripts that process/rename dist files ==="
cat libs/core/scripts/* 2>/dev/null | head -50
echo ""
echo "=== Look for rollup or webpack config that might transform output ==="
find libs/core -name "rollup.config.*" -o -name "webpack.config.*" 2>/dev/nullLength of output: 597
Critical issue: Package.json export paths don't match Stencil's actual output.
The exports reference incorrect CommonJS filenames that Stencil does not generate:
- Expected by package.json:
dist/index.cjs.jsandloader/index.cjs.js - Actually generated by Stencil:
dist/index.jsandloader/index.js
Stencil's 'dist' output target (configured in stencil.config.ts) produces .js files for CommonJS, not .cjs.js. This affects both the legacy "main" field (line 14) and the new "require" exports (lines 23, 26).
Required fixes:
- Line 14: Change
"main": "dist/index.cjs.js"→"dist/index.js" - Line 23: Change
"require": "./dist/index.cjs.js"→"./dist/index.js" - Line 26: Change
"require": "./loader/index.cjs.js"→"./loader/index.js"
🤖 Prompt for AI Agents
In libs/core/package.json around lines 14 and 19–26, the package "main" and
"require" export paths point to non-existent CommonJS filenames (*.cjs.js);
update "main" at line 14 from "dist/index.cjs.js" to "dist/index.js", update the
export "require" at line 23 from "./dist/index.cjs.js" to "./dist/index.js", and
update the loader export "require" at line 26 from "./loader/index.cjs.js" to
"./loader/index.js" so the fields match the actual Stencil output files.
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.
Re: Package export paths
Could your feedback be incorrect? Stencil does generate .cjs.js files for CommonJS output. Build verification confirms all referenced files exist:
✅ dist/index.cjs.js
✅ dist/esm/index.mjs
✅ dist/types/index.d.ts
✅ loader/index.cjs.js
The package.json export paths are correct as originally configured. Stencil even warns when using dist/index.js instead of dist/index.cjs.js for the main field.
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.
@pixelflips, you're absolutely correct, and I apologize for the incorrect feedback. Thank you for the clarification and for verifying the build output!
I was wrong to flag this as an issue. Since your build verification confirms that Stencil does indeed generate the .cjs.js files and all the export paths exist after build, the package.json configuration is correct as written. The export paths properly align with Stencil's actual output in your project.
I appreciate you taking the time to correct this with the evidence. The package.json exports configuration is good to go! 👍
(„• ֊ •„) taking notes carefully
✏️ Learnings added
Learnt from: pixelflips
Repo: Kajabi/pine PR: 592
File: libs/core/package.json:19-24
Timestamp: 2025-11-19T23:04:32.326Z
Learning: In the Pine monorepo (libs/core), Stencil generates `.cjs.js` files for CommonJS output (e.g., `dist/index.cjs.js`, `loader/index.cjs.js`), not just `.js` files. The package.json should reference these `.cjs.js` files in the "main" field and "require" exports.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
| @@ -1 +1,2 @@ | |||
| export * from './components'; | |||
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.
🧩 Analysis chain
Verify that the components module exports only public APIs.
This re-export makes all exports from ./components available at the package root, which should improve TypeScript resolution as intended. However, ensure that ./components exists and only exports public-facing component types and not internal implementation details.
Run the following script to verify the components module structure:
🏁 Script executed:
#!/bin/bash
# Verify ./components module exists and check its exports
# Check if components module exists
if [ -f "libs/core/src/components/index.ts" ]; then
echo "✓ components/index.ts exists"
echo "--- Exports from components/index.ts ---"
cat libs/core/src/components/index.ts
elif [ -f "libs/core/src/components.ts" ]; then
echo "✓ components.ts exists"
echo "--- Exports from components.ts ---"
cat libs/core/src/components.ts
else
echo "✗ Could not find components module at expected locations"
fd -e ts -e tsx "components" libs/core/src --max-depth 2
fiLength of output: 86
I need to investigate further. The script found only a .d.ts file but no actual implementation. Let me examine that file and check the module resolution configuration.
🏁 Script executed:
#!/bin/bash
# Examine the declaration file and module resolution
echo "--- Content of components.d.ts ---"
cat libs/core/src/components.d.ts
echo ""
echo "--- Checking tsconfig.json for path mappings ---"
if [ -f "tsconfig.json" ]; then
cat tsconfig.json | head -40
fi
echo ""
echo "--- Checking for package.json exports ---"
if [ -f "package.json" ]; then
cat package.json | grep -A 20 '"exports"'
fi
echo ""
echo "--- Directory listing of libs/core/src ---"
ls -la libs/core/src/ | head -20Length of output: 198066
I need to check if there's an actual index.ts file inside the components directory and what it exports.
🏁 Script executed:
#!/bin/bash
# Check for components/index.ts and its contents
if [ -f "libs/core/src/components/index.ts" ]; then
echo "✓ Found libs/core/src/components/index.ts"
echo "--- Content (first 100 lines) ---"
head -100 libs/core/src/components/index.ts
else
echo "✗ libs/core/src/components/index.ts NOT found"
echo ""
echo "--- Checking what files exist in components directory ---"
ls -la libs/core/src/components/ | head -20
fi
echo ""
echo "--- Checking libs/core/src/index.ts again for context ---"
cat libs/core/src/index.tsLength of output: 1354
The PR is missing the required implementation file libs/core/src/components/index.ts.
The re-export statement export * from './components' in libs/core/src/index.ts expects either a components.ts file or a components/index.ts directory index. The components/ directory exists with 37 component subdirectories (pds-accordion, pds-alert, pds-avatar, etc.), but there is no index.ts file inside it to re-export those components.
You must create libs/core/src/components/index.ts that re-exports all public APIs from the component subdirectories. Without this file, TypeScript module resolution will fail and the package export will be broken.
🤖 Prompt for AI Agents
In libs/core/src/components/index.ts around lines 1 to 1, create the missing
barrel file that re-exports each component subpackage so the top-level export in
libs/core/src/index.ts works; add exports for every component directory (e.g.
export * from './pds-accordion'; export * from './pds-alert'; export * from
'./pds-avatar'; … for all 37 subdirectories), ensuring you export the public
APIs from each component, and commit this new file so TypeScript module
resolution and package exports succeed.
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.
Re: Missing components/index.ts
The feedback tool misunderstood the module resolution. The statement export * from './components' in src/index.ts correctly resolves to the auto-generated src/components.d.ts file, not a directory index.
TypeScript automatically resolves ./components to ./components.d.ts when the .d.ts file exists. This file is generated by Stencil during build and contains all component type definitions.
Creating a manual src/components/index.ts barrel file would:
- Require manual maintenance when adding/removing components
- Duplicate what Stencil already generates automatically
- Defeat the purpose of using Stencil's type generation
The current implementation is correct and follows Stencil best practices.
Do you still think your proposed change is needed?
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.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify components.d.ts exists and examine its content
echo "--- Checking for components.d.ts ---"
if [ -f "libs/core/src/components.d.ts" ]; then
echo "✓ components.d.ts exists"
echo ""
echo "--- First 50 lines of components.d.ts ---"
head -50 libs/core/src/components.d.ts
echo ""
echo "--- Last 20 lines of components.d.ts ---"
tail -20 libs/core/src/components.d.ts
echo ""
echo "--- Total lines in file ---"
wc -l libs/core/src/components.d.ts
else
echo "✗ components.d.ts NOT found"
fi
echo ""
echo "--- Check for Stencil configuration ---"
if [ -f "libs/core/stencil.config.ts" ]; then
echo "✓ Found Stencil config"
cat libs/core/stencil.config.ts
fi
echo ""
echo "--- Check package.json for Stencil ---"
if [ -f "libs/core/package.json" ]; then
grep -i "stencil" libs/core/package.json || echo "No stencil references found"
fi
Length of output: 6581
🌐 Web query:
Stencil TypeScript import resolution components.d.ts best practices
💡 Result:
Here are concise best practices for resolving imports in the generated components.d.ts files for Stencil libraries (so consumers don’t get “could not find declaration” or resolution errors):
-
Don’t emit top-level imports of dev‑only types
- Avoid referencing types that live only as devDependencies in your generated .d.ts files. If a type is required at runtime or in consumer builds, it must be a dependency or peerDependency of your package. (Stencil’s distribution/output guidance and validation assume package.json fields point to usable outputs.) [1]
-
Prefer peerDependencies (with clear install instructions) for framework/host types
- For large framework typings (e.g., React/Vue) list them as peerDependencies and document that consumers must install the matching types (or provide a thin wrapper package that re-exports typed bindings). This avoids forcing consumers to install heavy runtime deps. [1][2]
-
Ship or re-export any third‑party typings your .d.ts references
- If your components.d.ts imports types from 3rd‑party packages, either:
a) move the type package from devDependency → dependency so the types are present when consumers install, or
b) include an ambient declaration or a small types file in your published package that declares (or re-exports) the needed symbols. - Many teams choose to bundle minimal .d.ts helpers or move @types/* to dependencies to prevent consumer build errors. [2]
- If your components.d.ts imports types from 3rd‑party packages, either:
-
Use ambient module declarations for JS-only deps you can’t type
- For small JS-only libs without types, add a project global .d.ts (e.g., src/types/globals.d.ts) with declare module 'module-name'; so generated .d.ts imports resolve to any rather than breaking consumer builds. Ensure that file is included in published types. (Standard TypeScript practice.) [3][4]
-
Use import type and avoid value imports in declaration files
- Where possible write import type { X } from 'pkg'; in your source so the compiler emits type-only references, reducing risk of introducing value imports that consumers can’t resolve. (Type-only imports are safer for declaration emission.)
-
Configure package.json "types" / exports correctly
- Ensure package.json points to the correct generated .d.ts (e.g., "types": "dist/index.d.ts" or via exports map) so consumers and TypeScript pick up your types automatically. Use Stencil’s dist/dist-types output targets and validation to help. [1]
-
Test consumption in a clean project
- Create a minimal consumer (vanilla TS, React/Vue) that installs your package from a local pack/tarball and verify no TS7016/TS2307 errors. This reveals missing dependencies or undeclared modules before publishing.
-
If you must reference many external types, provide a consumer-friendly facade
- Provide a small typed façade (e.g., exports/index.d.ts or React wrappers) that re-exports only the types consumers need, reducing direct cross-package imports inside generated components.d.ts. This also makes upgrading external deps easier. [1]
Quick implementation checklist
- Move any @types/* needed by emitted .d.ts into dependencies or bundle equivalent declarations in your package. [2]
- Add a global types file (included in package) to declare any untyped modules. [3]
- Use import type where possible and confirm package.json "types"/exports point to dist .d.ts. [1][3]
- Run a clean consume test before publishing.
Sources
- Stencil output targets / packaging & validation (distribution/types guidance). [1]
- Community discussion: 3rd‑party typings in Stencil — move @types from devDependency or provide typings. [2]
- TypeScript: ambient module declarations / declare module and type-only imports (practical fixes for "could not find declaration file"). [3][4]
@pixelflips You're absolutely correct, and I apologize for the confusion. My original review comment was incorrect.
After examining the codebase, I can confirm:
-
components.d.tsexists and is auto-generated by Stencil - The file contains 4,781 lines with the header stating "This is an autogenerated file created by the Stencil compiler." -
TypeScript correctly resolves
./componentsto./components.d.ts- This is standard TypeScript module resolution behavior when a.d.tsfile exists. -
Your implementation follows Stencil best practices - The
stencil.config.tsshows proper output target configuration, and Stencil automatically generates all component type definitions during the build process.
Creating a manual src/components/index.ts barrel file would indeed be redundant and defeat the purpose of Stencil's automatic type generation. The current implementation is correct.
Thank you for the clarification, and my apologies for the incorrect feedback. The change you've made is the right approach for improving TypeScript resolution in your Stencil component library.
✏️ Learnings added
Learnt from: pixelflips
Repo: Kajabi/pine PR: 592
File: libs/core/src/index.ts:1-1
Timestamp: 2025-11-19T23:09:31.597Z
Learning: In Stencil projects, `export * from './components'` correctly resolves to the auto-generated `components.d.ts` file. TypeScript automatically resolves module imports to `.d.ts` files when they exist. Do not suggest creating manual barrel files like `components/index.ts` as this would duplicate Stencil's automatic type generation.
Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.
|
@coderabbitai can you please do another final review based on the new information shared in our comment thread? |
|
@pixelflips: I'll perform another comprehensive review of the PR incorporating all the context from our discussion. Let me analyze the changes to the TypeScript type exports and the ✅ Actions performedReview triggered.
|
Description
Fixes TypeScript type resolution for Pine components. Previously, component types weren't properly exported from
@pine-ds/core, causing IDE issues and requiring developers to navigate type definitions manually.Changes:
src/index.ts)exportsfield topackage.jsonfor modern bundler/TypeScript supportThis improves IDE intellisense and type inference when using Pine components.
Fixes: DSS-13
Type of change
How Has This Been Tested?
Test Configuration:
kajabi-products) to verify:Checklist: