adds index.d.ts file next to index.cjs file #27
Conversation
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughThe pull request introduces modifications to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
|
View your CI Pipeline Execution ↗ for commit 154144c.
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/eslint-plugin-rxjs/project.json (2)
28-35: Simplify and strengthen the build commandWhile the command achieves the goal of placing index.d.ts next to index.cjs, it could be more robust.
Consider this improved command:
- "command": "echo 'Copying types...' && cp dist/packages/eslint-plugin-rxjs/src/index.d.ts dist/packages/eslint-plugin-rxjs" + "command": "cp -f dist/packages/eslint-plugin-rxjs/src/index.d.ts dist/packages/eslint-plugin-rxjs/"The
-fflag ensures the command won't fail if the file already exists, and removing the echo makes the command more concise.
48-50: Consider expanding lint patterns for TypeScript filesThe current lint patterns only include package.json, but for a TypeScript project, you might want to include .ts and .d.ts files as well.
Consider expanding the patterns:
"lintFilePatterns": [ - "packages/eslint-plugin-rxjs/package.json" + "packages/eslint-plugin-rxjs/package.json", + "packages/eslint-plugin-rxjs/**/*.ts", + "packages/eslint-plugin-rxjs/**/*.d.ts" ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
package.json(1 hunks)packages/eslint-plugin-rxjs/package.json(1 hunks)packages/eslint-plugin-rxjs/project.json(2 hunks)packages/eslint-plugin-rxjs/src/index.ts(1 hunks)packages/eslint-plugin-rxjs/tsconfig.lib.json(1 hunks)
🔇 Additional comments (6)
packages/eslint-plugin-rxjs/src/index.ts (1)
119-119: Confirm ES module usage consistency.
Switching to export default plugin; transitions the file to ES modules. Verify that all consumers still import the plugin correctly and that this aligns with the type: "commonjs" setting in the package.json. If your consumers use require(), they might need an interop flag or a different import style.
packages/eslint-plugin-rxjs/tsconfig.lib.json (1)
6-8: Validate NodeNext configuration compatibility.
Setting "module": "NodeNext" and "moduleResolution": "NodeNext" properly enables ES module support in Node.js environments. Confirm that downstream tooling and Node versions used in your build and runtime environments are compatible with NodeNext. Older Node versions might not fully support this approach.
packages/eslint-plugin-rxjs/package.json (1)
28-35: Ensure exports object meets all module consumer needs.
You have a "main" pointing to "./index.cjs" and an "exports" field with "require" and "types". Consider whether ESM users should also have an "import" entry or if a single CJS export is the intended usage. Verify this covers the consumption patterns you expect for your library, especially since index.ts is now using ES module syntax internally.
package.json (1)
6-6: Evaluate skipping Nx cache.
Using --skip-nx-cache may prevent using incremental builds and affect performance. Verify that skipping the cache is desired, as it can lead to slower build times. If the skip is only necessary under certain circumstances, consider restricting it to those scenarios instead of making it the default behavior.
packages/eslint-plugin-rxjs/project.json (2)
Line range hint 7-57: Verify the complete build pipeline for TypeScript declarations
The configuration looks correct for generating and placing TypeScript declarations. Let's verify the complete setup:
✅ Verification successful
TypeScript declarations setup is properly configured
The verification confirms that the build pipeline for TypeScript declarations is correctly set up:
- The package.json has proper TypeScript configuration:
"types": "./index.d.ts"is correctly specifiedexportsfield properly maps types via"types": "./index.d.ts"
- The source index.ts exists and exports the necessary components
- The build configuration in project.json correctly:
- Generates declarations with
"dts": true - Copies the generated declarations to the right location
- Bundles with proper format
"cjs"
- Generates declarations with
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the complete TypeScript declaration setup
# Check if package.json has proper type definitions configured
echo "Checking package.json type definitions configuration..."
cat packages/eslint-plugin-rxjs/package.json | jq 'select(.types, .typings, .exports.".")'
# Ensure source index.ts exists and exports properly
echo "Checking source index.ts exports..."
if [ -f "packages/eslint-plugin-rxjs/src/index.ts" ]; then
head -n 10 packages/eslint-plugin-rxjs/src/index.ts
fi
Length of output: 2371
7-27: Build configuration looks good and aligns with TypeScript declaration goals!
The build-lib target is well configured with:
- TypeScript declarations enabled via
dts: true - Proper bundling setup for distribution
- Correct CJS format specification
Let's verify the TypeScript configuration aligns with these build settings:
✅ Verification successful
Build and TypeScript configuration are properly aligned for declaration generation
The build configuration in project.json with dts: true is correctly supported by the TypeScript configuration in tsconfig.lib.json which has:
"declaration": trueenabled for generating .d.ts files- Proper module settings with
"module": "NodeNext" - Correct includes/excludes to process only the source files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if tsconfig.lib.json has proper declaration settings
cat packages/eslint-plugin-rxjs/tsconfig.lib.json | jq '.compilerOptions | select(.declaration == true and .declarationMap == true)'
Length of output: 134
Script:
#!/bin/bash
# Let's check the actual content of tsconfig.lib.json first
cat packages/eslint-plugin-rxjs/tsconfig.lib.json
Length of output: 387
DaveMBush
deleted the
dmb/missing-typescript-declaration-file-for-smarttoolseslint-plugin-rxjs/26
branch
Issue Number: #26
Body
Adds in missing d.ts file
Summary by CodeRabbit
Release Notes
Chores
New Features
Refactor