Skip to content

Remove our use of default exportsΒ #945

@rowanmanning

Description

@rowanmanning

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:

- const registerCrashHandler = require('@dotcom-reliability-kit/crash-handler');
+ const { registerCrashHandler } = require('@dotcom-reliability-kit/crash-handler');

- import registerCrashHandler from '@dotcom-reliability-kit/crash-handler';
+ import { registerCrashHandler } from '@dotcom-reliability-kit/crash-handler';

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, and serialize-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:

  • eslint-config
  • fetch-error-handler
  • opentelemetry
  • app-info
  • crash-handler
  • errors
  • log-error
  • logger
  • middleware-log-errors
  • middleware-render-error-info
  • serialize-error
  • serialize-request

Metadata

Metadata

Assignees

No one assigned

    Labels

    breaking changeIndicates that the feature will require a major version bump.enhancementNew feature or request

    Type

    No type

    Projects

    Status

    πŸ“ Planned

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions