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

feat: transform bauta into an ESM module #165

Draft
wants to merge 31 commits into
base: main
Choose a base branch
from

Conversation

Xavier-Redondo
Copy link
Contributor

@Xavier-Redondo Xavier-Redondo commented May 28, 2024

💣 💣 💣 💣 💣 💣

Do not merge this PR into main

The goal of this PR is to check the changes before generating a beta or alpha release, not merging into main.

💣 💣 💣 💣 💣 💣

BREAKING CHANGE

Note: this is going to be a VERY BREAKING_CHANGE because when using bauta as an ESM module it will force your own project itself to be an ESM module (you can check details in the example services in this same pr).

However, besides this change you will not be required to change anything else at bauta level to make it work. As a limit case, if you were using an already ESM module project with BautaJs, for you it would not be a breaking change (But this is a limit case and we assume that for most if not all cases you will need to change your project from CJS to ESM).

PR Checklist

  • I have run npm test locally and all tests are passing.
  • I have added/updated tests for any new behavior.
  • I have added/updated documentation for any new behavior. --> NO, THIS IS NOT DONE YET because i need to check first a lot of stuff and for that i need a beta release.
  • If this is a significant change, an issue has already been created where the problem / solution was discussed: [N/A, or add link to issue here]

PR Description

This PR description is a bit long because details what i have done to adapt the bauta.js code to ESM modules. Note that there is no other change at all in this PR. Thus, i have not changed jest, i have not updated any library whatsoever and any change you see in this PR is required directly or indirectly to make bauta work as an ESM module.

As a "I have no idea what I am doing" starting point guide i have used this link.

A good comparison between CJS and ESM is here (but this does not explain anything related to what this PR is doing).

Changes in the code

Basic required trivial changes

  • updated all the package.json accordingly:
    • change/add exports field
    • set type to module.
    • update all scripts to use the flag experimental-vm-modules when executing node (either for test or for start anything).
  • remove any require from the code and transform them into import. Check section below for the only non-trivial case.
  • all dependency from node has to be namespaced with node:. Example: in a module you do not import path but node:path.
  • all internal dependency must have extension js. Example: you do not import './index' but '.index.js' (even in the typescript files).
  • make the modifications required in tsconfig.base.json to compile the code to ESM and not to node using require javascript.
  • When using an import that comes from a default export, i have had to add .default to certain calls to make it work.
  • any module.exports has to be changed, either to the named export formula export {stuff } or to the default export formula export default stuff

required non-trivial change: change require to import when loading resolvers

There is a single occurrence of a require that is not trivial and whose transformation in require has implications.

  1. bauta.ts file, function requireAll: this function was using synchronously require to load the resolvers.
  2. To load dynamically stuff with import, the import has to be asynchronously.
  3. Thus, this single change forces the requireAll function to change from synchronous to asynchronous.
  4. This has a chain effect on certain parts, the most important part is the constructor of BautaJS since you cannot have asynchronous code in a constructor.
  5. I have moved the code that was inside this constructor and dealt with the loading of the resolvers to a new asynchronous function called loadResolvers.
  6. I have created two new private fields called resolversPath and resolvers to act as a bridge between the moment that we are instantiating bauta and the moment when asynchronously we are loading the resolvers.
  7. I have created a flag resolversAlreadyLoaded to make sure that we are not loading the resolvers twice. This is required because if using versioning, when using the function inheritOperationsFrom we need to load the resolvers before bootstrapping to work correctly.
  8. I have updated inheritOperationsFrom to load the resolvers.
  9. I have updated the constructor of express BautaJSExpress to bootstrap before the middleware application, otherwise the routes were not loaded correctly because there was not resolvers on it.

So basically as a summary:

  • Before: At time on instantiating BautaJS you had your resolvers instantiated synchronously. They were not yet on any route, but they were ready to be used by bauta to load the routes, and most importantly they could be played from inside inheritOperationsFrom function if using versioning.

  • Now: After instantiating BautaJS your resolvers are still totally and utterly empty (new private variable resolvers). The only change is that the private variable resolversPath has the value of where the resolvers are. Only after either calling bootstrap (normal usage) or inheritOperationsFrom (usage with versioning), are the resolvers asynchronously loaded through the new internal method loadResolvers.

Changes in the test

For changes into tests you may need to check jest documentation here and ts-jest documentation here.

On top of the trivial changes already described, for test files i have had to do the following:

  • the code dealing with the ts-jest transformation to use esm, through adding a moduleNameMapper, plus the new preset and the use of the flag useESM. This is basically the changes in the file jest.config.base.js.
  • since the loading of the ts-jest preset from the different projects must use require, i have had to change all the file extension for jest.config.js to jest.config.cjs.
  • a lot of tests were using __dirname to load the resolvers when testing. In an ESM module, the __dirname no longer exists and i have made an utility function that simulates the __dirname.
  • I think that i increased a version of typescript and now typing is more strict (i have had to add extra castings and correct a few typing stuff). This is not related to ESM though.
  • When using jest you have to import accordingly from @jest/globals

Copy link

github-actions bot commented May 28, 2024

Dependency Review

The following issues were found:
  • ✅ 0 vulnerable package(s)
  • ✅ 0 package(s) with incompatible licenses
  • ✅ 0 package(s) with invalid SPDX license definitions
  • ⚠️ 4 package(s) with unknown licenses.
See the Details below.

License Issues

packages/bautajs-datasource-rest/package.json

PackageVersionLicenseIssue Type
@axa/bautajs-dev-config^4.0.0-alpha.0NullUnknown License
@axa/bautajs-core^4.0.0-alpha.0NullUnknown License

packages/bautajs-express/package.json

PackageVersionLicenseIssue Type
@axa/bautajs-core^4.0.0-alpha.0NullUnknown License

packages/bautajs-fastify/package.json

PackageVersionLicenseIssue Type
@axa/bautajs-core^4.0.0-alpha.0NullUnknown License
Denied Licenses: GPL-2.0+, AGPL-3.0-or-later, LGPL-2.0-or-later

Scanned Manifest Files

packages/bautajs-core/package.json
  • @axa/bautajs-dev-config@^4.0.0-alpha.0
  • fast-glob@^3.3.0
  • @axa/bautajs-dev-config@^3.0.1
  • fast-glob@^3.2.12
packages/bautajs-datasource-rest/package.json
  • @axa/bautajs-core@^4.0.0-alpha.0
  • @axa/bautajs-dev-config@^4.0.0-alpha.0
  • @axa/bautajs-core@^3.1.0
  • @axa/bautajs-dev-config@^3.0.1
packages/bautajs-express/package.json
  • @axa/bautajs-core@^4.0.0-alpha.0
  • @axa/bautajs-dev-config@^4.0.0-alpha.0
  • @axa/bautajs-core@^3.1.0
  • @axa/bautajs-dev-config@^3.0.1
packages/bautajs-fastify/package.json
  • @axa/bautajs-core@^4.0.0-alpha.0
  • @axa/bautajs-datasource-rest@^4.0.0-alpha.0
  • @axa/bautajs-dev-config@^4.0.0-alpha.0
  • @axa/bautajs-core@^3.1.0
  • @axa/bautajs-datasource-rest@^3.1.0
  • @axa/bautajs-dev-config@^3.0.1

@Xavier-Redondo
Copy link
Contributor Author

do not merge yet into master or you will make my cats sad 🐈 🐈

@Xavier-Redondo Xavier-Redondo changed the title feat: transform bauta into a module feat: transform bauta into an ESM module May 29, 2024
@franher franher added the 4.x label May 29, 2024
@franher
Copy link
Contributor

franher commented May 29, 2024

Thank you for your contribution, @Xavier-Redondo. I will review the code ASAP and prepare an alpha version. However, I have a question about the implications of this change for a Bauta.js user. I see it will be a breaking change for them, right? Can you elaborate on this topic in the summary? Thank you

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants