Skip to content
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

Add CommonJS support for js-sdk and fix TypeScript definitions for better IDE support in platforms like Nest.js #9229

Open
wants to merge 2 commits into
base: develop
Choose a base branch
from

Conversation

centrual
Copy link

What

This PR focuses on improving the JavaScript SDK by adding CommonJS support and fixing TypeScript type issues. Changes include:

  • CommonJS support: Added support for CommonJS modules to enhance compatibility with platforms like Nest.js.
  • TypeScript types dependency: Added necessary TypeScript type packages as dependencies to ensure the SDK can compile properly with TypeScript support.
  • Header normalization fix: Corrected an issue where headers were incorrectly formatted, leading to improper merging.
  • Return statement cleanup: Removed unnecessary return statements in client methods for code clarity and consistency.
  • Build script update: Updated the build script to generate both ES modules and CommonJS outputs.
  • Dependency updates: Updated and pinned versions for Jest, MSW, and TypeScript to ensure stability and prevent unexpected updates.

Why

These changes are essential to resolve issues with TypeScript type definitions in the Medusa JS SDK, allowing it to properly compile and function with TypeScript support. Additionally, they ensure compatibility with CommonJS-based platforms like Nest.js and provide a more stable development environment.

  • CommonJS support: Ensures compatibility with frameworks like Nest.js that rely on CommonJS.
  • TypeScript type dependencies: Added required packages to prevent compilation issues and improve IDE support.
  • Dependency updates: Pinned versions help avoid unexpected package updates that could introduce breaking changes.

How

The changes were implemented by:

  • Adding CommonJS support in the package.json through the module and exports fields.
  • Adding necessary TypeScript type packages as dependencies to ensure the SDK compiles correctly.
  • Updating the build scripts to output both CommonJS and ES modules.
  • Normalizing headers in client request handling to ensure correct merging.
  • Cleaning up return statements in client methods.
  • Pinning dependency versions for Jest, MSW, and TypeScript to prevent unexpected updates and ensure stability.

Testing

These changes were tested using the existing test suite, which includes:

  • Unit tests: Covering client request/response behavior and TypeScript type validation.
  • Manual testing: Verified that the SDK compiles correctly with the added TypeScript dependencies and works as expected in a CommonJS environment like Nest.js.

Additional manual tests were performed to validate the correct behavior of the CommonJS integration and dependency updates.

…ern JavaScript environments

chore(js-sdk): bump version to 0.0.2 to reflect new ES module changes
chore(js-sdk): update "node" engine requirement to ">=20" as GitHub workflows now use Node.js 20 as the minimum version
fix(js-sdk): normalize headers in client requests to fix incorrect merging due to wrong return format (should return a JSON object, not Headers class)
chore(js-sdk): remove unnecessary return statements in client methods for code clarity and consistency
chore(js-sdk): adjust TypeScript config to support both ESNext and CommonJS modules
chore(js-sdk): update build script to generate ES module and CommonJS outputs
chore(js-sdk): update dependencies for Jest, MSW, and TypeScript to pin versions and prevent unexpected package updates
Copy link

changeset-bot bot commented Sep 22, 2024

⚠️ No Changeset found

Latest commit: aa3d8c4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link

vercel bot commented Sep 22, 2024

Someone is attempting to deploy a commit to the medusajs Team on Vercel.

A member of the Team first needs to authorize it.

@centrual centrual changed the title Add CommonJS support and fix TypeScript definitions for better IDE support in platforms like Nest.js Add CommonJS support for js-sdk and fix TypeScript definitions for better IDE support in platforms like Nest.js Sep 22, 2024
@SalahAdDin
Copy link

It should be better has the config file also in ts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants