Financial-Times/dotcom-reliability-kit

Write tests to increase confidence in different module systems

Closed this issue · 2 comments

rowanmanning commented

We need a test suite that increases our confidence that Reliability Kit works regardless of which module system our consuming apps use.

What problem does this feature solve?

We've had a bit of a nightmare with supporting different module systems. Many of our issues have been due to type hinting being wrong or modules not being importable as we expected based on the way an app is set up. I think we need to increase our confidence that Reliability Kit will work across the majority of our Node.js projects. Ideally we should do this without introducing too much overhead.

Ideal solution

Write a custom eslint plugin that would check our reliability-kit .js files for the presence of a module.exports and if present, it should also have a corresponding module.exports.default. If the module.exports.default is not present then eslint will error and the commit will fail as part of the husky process.

Alternatives

We should write some end-to-end tests, either at the package level or in the root of the project, which imports and checks each of the expected public exports. We need each module to work in at least the following:

  • In a CommonJS app (non-compiled)
  • In an ESM app (non-compiled)
  • In a TypeScript app that outputs CommonJS (esModuleInterop set to false)
  • In a TypeScript app that outputs ESM (esModuleInterop set to true)
  • In a TypeScript app that outputs CommonJS (esModuleInterop set to false)
  • In a TypeScript app that outputs ESM (esModuleInterop set to true)
  • In a CommonJS app which uses Sucrase
  • In a CommonJS app which is compiled with Babel

We could also do nothing, but I'm finding that my development is slowed down because I have to manually test in a few different module systems locally to be sure I'm not introducing a breaking change.

rowanmanning commented

I'm unsure if this is as high a priority right now as we haven't had any issues despite adoption across the group increasing. Gonna move back to the inbox for now and we can pick up later if the logger roll-out surfaces any issues.

rowanmanning commented

I'm going to consider this as done, we have tests in place and we'll add new ones for the other modules once we switch to manual type declarations.