-
Notifications
You must be signed in to change notification settings - Fork 1
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
Remove our use of default exports #945
Labels
breaking change
Indicates that the feature will require a major version bump.
enhancement
New feature or request
Milestone
Comments
rowanmanning
added
enhancement
New feature or request
breaking change
Indicates that the feature will require a major version bump.
labels
Feb 21, 2024
rowanmanning
changed the title
Remove our use of default exports.
Remove our use of default exports
Feb 21, 2024
rowanmanning
added a commit
that referenced
this issue
Jun 12, 2024
This is going to cause us lots of problems when we want to export new methods, e.g. methods to send custom metrics which I'll be doing in a later commit. Nobody is using the JavaScript API at the moment and so this is a safe major version bump for most. It's also worth addressing this while use is still low. See #945.
rowanmanning
added a commit
that referenced
this issue
Jun 12, 2024
This is going to cause us lots of problems when we want to export new methods, e.g. methods to send custom metrics which I'll be doing in a later commit. Nobody is using the JavaScript API at the moment and so this is a safe major version bump for most. It's also worth addressing this while use is still low. See #945.
rowanmanning
added a commit
that referenced
this issue
Jun 12, 2024
This is going to cause us lots of problems when we want to export new methods, e.g. methods to send custom metrics which I'll be doing in a later commit. Nobody is using the JavaScript API at the moment and so this is a safe major version bump for most. It's also worth addressing this while use is still low. See #945.
rowanmanning
added a commit
that referenced
this issue
Jun 12, 2024
This is going to cause us lots of problems when we want to export new methods, e.g. methods to send custom metrics which I'll be doing in a later commit. Nobody is using the JavaScript API at the moment and so this is a safe major version bump for most. It's also worth addressing this while use is still low. See #945.
rowanmanning
added a commit
that referenced
this issue
Jun 19, 2024
This is going to cause us lots of problems when we want to export new methods, e.g. methods to send custom metrics which I'll be doing in a later commit. Nobody is using the JavaScript API at the moment and so this is a safe major version bump for most. It's also worth addressing this while use is still low. See #945.
rowanmanning
added a commit
that referenced
this issue
Jun 19, 2024
This is going to cause us lots of problems when we want to export new methods, e.g. methods to send custom metrics which I'll be doing in a later commit. Nobody is using the JavaScript API at the moment and so this is a safe major version bump for most. It's also worth addressing this while use is still low. See #945.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Labels
breaking change
Indicates that the feature will require a major version bump.
enhancement
New feature or request
Default exports cause a lot of issues with the package types, we should stop using them and use named exports across any Reliability Kit modules we expect to be imported (i.e. all except
eslint-config
). This would be a breaking change.What problem does this feature solve?
Default exports cause many of the compatibility issues between CommonJS and ESM. They also require us to make adjustments to get the types to work properly. If we weren't using default exports then we'd sidestep some of the issues with TypeScript and
esModuleInterop
, effectively reducing the number of module systems we test against by two.If we stop using default exports then it's also a lot easier to start writing our code as ESM and cross-compiling to CommonJS (something I've experimented with but got blocked by our default exports). Writing as ESM feels more future-proof.
Ideal solution
We move all modules to used named exports, which would require code changes in downstream apps like this:
Although it requires a code change, it's very find/replaceable and we could do it with codemods (something that the Developer Automation team have been looking into).
If we think it's worthwhile, we could also introduce named exports to the modules as aliases in the short-term so that teams can start migrating ahead of the breaking change.
Alternatives
We could continue to use default exports but we'll be making it harder to migrate to ESM and we'd be accepting continued maintenance of several module compatibility tests that we could otherwise remove.
We could also start doing this work earlier for our low-level modules, e.g.
app-info
,serialize-request
, andserialize-error
– these modules are used by apps less frequently and mostly used in our own higher-level modules.Checklist
These are the packages that need migrating. Some of them will just be a case of removing the
default
alias:The text was updated successfully, but these errors were encountered: