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

fix(ESM): support mts and cts files #1455

Merged
merged 4 commits into from
Jul 30, 2023
Merged

Conversation

Kampfmoehre
Copy link
Contributor

@Kampfmoehre Kampfmoehre commented Jul 27, 2023

All Submissions:

Closing issues
#1307

If this is a new feature submission:

  • Has the issue had a maintainer respond to the issue and clarify that the feature is something that aligns with the goals and philosophy of the project?

Potential Problems With The Approach

Haven't had the ability to also test .cts files but I think this should be straight forward.

Test plan

I added two unit tests for the route generator method getRelativeImportPath that replaces the extension in controller files.

private setProgramToDynamicControllersFiles(controllers: string[]) {
const allGlobFiles = importClassesFromDirectories(controllers);
private setProgramToDynamicControllersFiles(controllers: string[], esm: boolean) {
const allGlobFiles = importClassesFromDirectories(controllers, esm ? ['.mts', '.ts', '.cts']: ['.ts']);
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does it hurt to add these in case we are CJS?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it hurts. As far as I know you have 3 possible ways to work with ESM in TypeScript:

  • keep ts files
  • use mts files when you want to output specifically ESM files
  • use cts files when you also want to output CommonJS

For our use case we only need mts/mjs files, but the logic is the same for the others as far as I know.
For reference: https://www.typescriptlang.org/docs/handbook/esm-node.html#new-file-extensions

@WoH WoH merged commit acfcc43 into lukeautry:master Jul 30, 2023
27 checks passed
@Kampfmoehre Kampfmoehre deleted the esm-fix branch August 4, 2023 10:07
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.

None yet

2 participants