-
Notifications
You must be signed in to change notification settings - Fork 227
Add ts support #920
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Add ts support #920
Conversation
integration test code coverage
Coverage Breakdown • (19%)
|
unit test code coverage
Coverage Breakdown • (89%)
|
…nto feature/ts-support
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds TypeScript support to the project by introducing type definitions and converting the main entry point to TypeScript. The changes enable better type safety and developer experience when using this OpenAPI to Postman converter library.
Key changes:
- New TypeScript type definitions in
types/index.tscovering all major interfaces - Conversion of
index.jstoindex.tswith TypeScript annotations - Build pipeline setup with TypeScript compilation to
dist/directory
Reviewed changes
Copilot reviewed 25 out of 35 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| types/index.ts | Comprehensive type definitions for converter inputs, options, and outputs |
| index.ts | TypeScript conversion of main entry point with type annotations |
| package.json | Added TypeScript tooling, build scripts, and updated main/types entry points |
| libV2/index.js | Updated import paths to reference reorganized CollectionGeneration directory |
| libV2/CollectionGeneration/schemaUtils.js | Added missing eslint-disable comment and fixed import paths with .js extensions |
| libV2/CollectionGeneration/validationUtils.js | Updated import paths with .js extensions for relocated modules |
| test/unit/*.test.js | Updated test imports to use compiled dist/index.js instead of source |
| test/system/*.test.js | Updated test imports to use compiled dist/index.js |
| test/integration/integration.test.js | Updated import to use compiled dist/index.js |
| scripts/test-lint.sh | Removed index.js from eslint targets since it's now TypeScript |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { MODULE_VERSION } = require('../lib/schemapack.js'); | ||
| const SchemaPack = require('../lib/schemapack.js').SchemaPack; | ||
| const UserError = require('../lib/common/UserError'); |
Copilot
AI
Dec 29, 2025
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.
The require paths use '../lib/' but should use './lib/' since index.ts is at the project root, not in a subdirectory. These incorrect paths will cause module resolution failures at runtime.
| const { MODULE_VERSION } = require('../lib/schemapack.js'); | |
| const SchemaPack = require('../lib/schemapack.js').SchemaPack; | |
| const UserError = require('../lib/common/UserError'); | |
| const { MODULE_VERSION } = require('./lib/schemapack.js'); | |
| const SchemaPack = require('./lib/schemapack.js').SchemaPack; | |
| const UserError = require('./lib/common/UserError'); |
| else if (propValue.properties) { | ||
| let processedProperties = processSchema(propValue); | ||
| propertyDetails.properties = processedProperties.properties; | ||
| // eslint-disable-next-line max-depth |
Copilot
AI
Dec 29, 2025
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.
The added eslint-disable comment suggests deeply nested code at this location. Consider refactoring to reduce nesting depth rather than suppressing the warning, as excessive nesting impacts code readability and maintainability.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 28 out of 38 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const { MODULE_VERSION } = require('../lib/schemapack.js'); | ||
| const SchemaPack = require('../lib/schemapack.js').SchemaPack; | ||
| const UserError = require('../lib/common/UserError'); |
Copilot
AI
Dec 30, 2025
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.
The require statements use relative paths starting with '../lib/' but these files are now at './lib/' relative to index.ts in the project root. These paths will fail at runtime since the compiled JavaScript will be in the dist directory.
| const { MODULE_VERSION } = require('../lib/schemapack.js'); | |
| const SchemaPack = require('../lib/schemapack.js').SchemaPack; | |
| const UserError = require('../lib/common/UserError'); | |
| const { MODULE_VERSION } = require('./lib/schemapack.js'); | |
| const SchemaPack = require('./lib/schemapack.js').SchemaPack; | |
| const UserError = require('./lib/common/UserError'); |
| return schema.detectRelatedFiles(); | ||
| }, | ||
|
|
||
| bundle: async function(input: MultiFileSpecInput & { options?: Options }): Promise<BundledContent> { |
Copilot
AI
Dec 30, 2025
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.
The parameter type uses intersection with an inline type which is inconsistent with the defined MultiFileSpecInput interface. Consider adding an 'options' field directly to MultiFileSpecInput or creating a separate BundleInput type.
No description provided.