-
Notifications
You must be signed in to change notification settings - Fork 3
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
Xavier-Redondo
wants to merge
31
commits into
main
Choose a base branch
from
feat/experiment-esm
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…t/experiment-got
…t/experiment-esm
…t/experiment-esm
Dependency ReviewThe following issues were found:
License Issuespackages/bautajs-datasource-rest/package.json
packages/bautajs-express/package.json
packages/bautajs-fastify/package.json
Denied Licenses: GPL-2.0+, AGPL-3.0-or-later, LGPL-2.0-or-later Scanned Manifest Filespackages/bautajs-core/package.json
packages/bautajs-datasource-rest/package.json
packages/bautajs-express/package.json
packages/bautajs-fastify/package.json
|
do not merge yet into master or you will make my cats sad 🐈 🐈 |
Xavier-Redondo
changed the title
feat: transform bauta into a module
feat: transform bauta into an ESM module
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
💣 💣 💣 💣 💣 💣
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
npm test
locally and all tests are passing.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
exports
fieldtype
tomodule
.experimental-vm-modules
when executing node (either for test or for start anything).require
from the code and transform them intoimport
. Check section below for the only non-trivial case.node:
. Example: in a module you do not importpath
butnode:path
.'./index'
but'.index.js'
(even in the typescript files)..default
to certain calls to make it work.module.exports
has to be changed, either to the named export formulaexport {stuff }
or to the default export formulaexport default stuff
required non-trivial change: change
require
toimport
when loading resolversThere is a single occurrence of a require that is not trivial and whose transformation in require has implications.
resolversPath
andresolvers
to act as a bridge between the moment that we are instantiating bauta and the moment when asynchronously we are loading the resolvers.resolversAlreadyLoaded
to make sure that we are not loading the resolvers twice. This is required because if using versioning, when using the functioninheritOperationsFrom
we need to load the resolvers before bootstrapping to work correctly.inheritOperationsFrom
to load the resolvers.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 variableresolversPath
has the value of where the resolvers are. Only after either callingbootstrap
(normal usage) orinheritOperationsFrom
(usage with versioning), are the resolvers asynchronously loaded through the new internal methodloadResolvers
.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:
useESM
. This is basically the changes in the filejest.config.base.js
.jest.config.js
tojest.config.cjs
.__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
.@jest/globals