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

chore: migrate src/pipetransport to typescript #5692

Merged
merged 2 commits into from Apr 20, 2020

Conversation

jackfranklin
Copy link
Collaborator

Hit one bump in the fact that I want to share an interface across files.
TypeScript only lets you import/export these if you're using ESM, not
CommonJS. So the two options are:

  • Migrate to ESM on a per file basis as we do this migration. This won't
    affect the output as we output as CommonJS.
  • Create a global types.d.ts file that we'll use and then migrate to
    ESM after.

Right now I've gone for the second option in order to not introduce more
changes in one go. But if we end up finding we have lots of
interfaces/types/etc that we want modules to expose, we might decide
slowly introducing ESM might be a better way forwards.

Hit one bump in the fact that I want to share an interface across files.
TypeScript only lets you import/export these if you're using ESM, not
CommonJS. So the two options are:

- Migrate to ESM on a per file basis as we do this migration. This won't
affect the output as we output as CommonJS.
- Create a global `types.d.ts` file that we'll use and then migrate to
ESM after.

Right now I've gone for the second option in order to not introduce more
changes in one go. But if we end up finding we have lots of
interfaces/types/etc that we want modules to expose, we might decide
slowly introducing ESM might be a better way forwards.
src/types.d.ts Outdated Show resolved Hide resolved
Copy link
Member

@mathiasbynens mathiasbynens left a comment

Choose a reason for hiding this comment

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

I would have been fine with option 1 too, FWIW. Happy to go with your recommendation though!

@jackfranklin
Copy link
Collaborator Author

@mathiasbynens 👍 It might be that we realise soon option 1 is better in which case let's just swap. One isn't more work than the other really so I'll just see how we go

Co-Authored-By: Mathias Bynens <mathias@qiwi.be>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants