feat(mama): implement moduleType getter along with inspectModuleType fn#397
Conversation
🦋 Changeset detectedLatest commit: 6d68b9b The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances module type detection by introducing a new getter in ManifestManager and a corresponding inspectModuleType utility function, along with comprehensive tests and updated documentation.
- Adds a new inspectModuleType utility to determine module types (dts, faux, dual, esm, cjs) based on various package.json properties.
- Implements a new moduleType getter in ManifestManager and provides tests to validate its behavior.
- Updates public exports and documentation to include the new moduleType functionality.
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| workspaces/mama/test/inspectModuleType.spec.ts | Adds tests covering various scenarios for inspectModuleType. |
| workspaces/mama/test/ManifestManager.spec.ts | Introduces a test for the moduleType getter in ManifestManager. |
| workspaces/mama/src/utils/inspectModuleType.ts | Implements the inspectModuleType function with recursive flag setting. |
| workspaces/mama/src/utils/index.ts | Re-exports inspectModuleType and associated types. |
| workspaces/mama/src/index.ts | Updates module exports to include inspectModuleType and its type. |
| workspaces/mama/src/ManifestManager.class.ts | Adds the moduleType getter using inspectModuleType. |
| workspaces/mama/README.md | Adds a documentation section for moduleType. |
| .changeset/crazy-drinks-sleep.md | Provides a changeset note for the module type detection implementation. |
| } | ||
|
|
||
| // Explicit `commonjs` set, with a explicit `import` or `.mjs` too. | ||
| if (esm && type === "commonjs") { |
There was a problem hiding this comment.
The conditions on lines 39-46 check flags (esm and cjs) that may not be initialized yet, which could lead to unexpected behavior. Consider reviewing or reordering these conditions to ensure that they correctly reflect the intended logic.
| .some((script) => kUnsafeNPMScripts.has(script.toLowerCase())); | ||
| } | ||
|
|
||
| get moduleType() { |
There was a problem hiding this comment.
[nitpick] Consider adding an explicit return type annotation for the moduleType getter to improve readability and type clarity.
| get moduleType() { | |
| get moduleType(): ReturnType<typeof inspectModuleType> { |
|
|
||
| Return the type of the module | ||
|
|
||
| ```ts |
There was a problem hiding this comment.
[nitpick] The documentation for moduleType is brief; consider expanding it to list the possible return values ('dts', 'faux', 'dual', 'esm', 'cjs') and provide a short explanation for each.
close #383