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: add ESM support #8536

Merged
merged 16 commits into from Jan 31, 2022
Merged

Conversation

giladgd
Copy link
Contributor

@giladgd giladgd commented Jan 17, 2022

Description of change

  • Support importing TypeORM in ESM projects
  • Support loading ESM entity and connection config files
  • Solution to circular dependency imports in ESM projects
  • Added support for generating base ESM projects using the typeorm init command
  • New section in the docs about ESM
  • Updated the docs to include examples about how to use the CLI in ESM projects

Added index.mjs to the package, so ESM imports will work.
index.mjs exposes the rest of the project which still uses commonjs.

Improved the implementation of dynamic file importing, the new implementation uses a function that determines the module system of the file and then uses require or import as necessary.

Fixes #6974
Fixes #6941
Fixes: #7516
Fixes: #7159

Improves #8530

Pull-Request Checklist

  • Code is up-to-date with the master branch
  • npm run lint passes with this change
  • npm run test passes with this change
  • This pull request links relevant issues as Fixes #0000
  • There are new or updated unit tests validating the change
  • Documentation has been updated to reflect this change
  • The new commits follow conventions explained in CONTRIBUTING.md

@giladgd giladgd changed the title ✨ [Feature] Support importing TypeORM in ESM projects feat: support importing TypeORM in ESM projects Jan 17, 2022
@giladgd giladgd force-pushed the supportImportingFromEsmProjects branch from 9b76f2a to eae1c03 Compare January 17, 2022 02:52
@pleerock
Copy link
Member

I would like to get a confirmation if these changes aren't breaking.

@pleerock
Copy link
Member

also @imnotjames maybe you want to review this one since I don't really have experience working with ESM yet.

The new implementation generates ESM exports directly out of the commonjs exports, and provides a default export to maintain compatability with existing `import`s of the commonjs implementation
@giladgd
Copy link
Contributor Author

giladgd commented Jan 17, 2022

@pleerock I've changed the implementation to a more custom and safe approach to make sure it maintains compatibility with current usages in both commonjs and ESM projects.
I've tested importing the new package from both commonjs and ESM projects and I confirm it doesn't break any import, it only adds the ability to import more than just the default export in ESM projects.

Previously you would have needed to do the following in ESM projects:

import typeorm from "typeorm";
const {createConnection} = typeorm;
import type {Connection, Logger} from "typeorm";

After this PR you would be able to also do this instead:

import {createConnection, Connection, Logger} from "typeorm";

The previous example will still continue to work

@KevinNovak
Copy link

I just converted my project to ESM and ran into an error:
SyntaxError: The requested module 'typeorm' does not provide an export named 'BaseEntity'

I am attempting to import with import { BaseEntity } from 'typeorm';

Would this solve this situation?

@giladgd
Copy link
Contributor Author

giladgd commented Jan 17, 2022

@KevinNovak Yes, this is the purpose of this PR

@KevinNovak
Copy link

@giladgd Thank you, looking forward to this update!

When TypeORM tries to load an entity file or a connection config file, it will determine what is the appropriate module system to use for the file and then `import` or `require` it as it sees fit.

Closes: typeorm#7516
Closes: typeorm#7159
@giladgd giladgd changed the title feat: support importing TypeORM in ESM projects feat: add ESM support Jan 18, 2022
@KevinNovak
Copy link

KevinNovak commented Jan 18, 2022

Thanks for working on the circular dependency fix too. I have entities that have a @OneToMany and @ManyToOne with each other and just ran into the circular dependency issue that I've seen happens with ESM. Will have to test out this change.

@KevinNovak
Copy link

I compiled your branch and tested it against my ESM project.

  • I can import typeorm normally! No need for workaround syntax that was previously needed.
  • Your "Related" change completely solved my circular dependency issue! I can keep using the decorator syntax.

Amazing work, thank you for this!

@giladgd
Copy link
Contributor Author

giladgd commented Jan 19, 2022

@KevinNovak Thank you for sharing :)
I hope this PR will be merged soon so ESM projects wouldn't give up on TypeORM.

@@ -96,9 +103,9 @@ export class InitCommand implements yargs.CommandModule {
// Protected Static Methods
// -------------------------------------------------------------------------

protected static executeCommand(command: string) {
protected static executeCommand(command: string, cwd: string) {
Copy link
Contributor Author

@giladgd giladgd Jan 19, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Previously, npm install command failed when a new folder was created for the new project

docs/faq.md Outdated
}
```

Doing this prevents the type of the key from being saved in the transpiled code in the key metadata, preventing circular dependency imports.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you please explain me what exact problem with the "key" stored in the transpiled code and what is called key? Is it "profile"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's say we have these TS ESM files:

// Profile.ts

import {Entity, OneToOne} from "typeorm";
import User from "./User.js";

@Entity()
export default class Profile {
    @OneToOne(() => User, user => user.profile)
    user: User;
}
// User.ts

import {Entity, OneToOne} from "typeorm";
import Profile from "./Profile.js";

@Entity()
export default class User {
    @OneToOne(() => Profile, profile => profile.user)
    profile: Profile;
}

Their compiled code will look like this:

Profile.js
// Profile.js

var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
    var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d;
    if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);
    else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
    return c > 3 && r && Object.defineProperty(target, key, r), r;
};
var __metadata = (this && this.__metadata) || function (k, v) {
    if (typeof Reflect === "object" && typeof Reflect.metadata === "function") return Reflect.metadata(k, v);
};
import { Entity, OneToOne } from "typeorm";
import User from "./User.js";
let Profile = class Profile {
};
__decorate([
    OneToOne(() => User, user => user.profile),
    __metadata("design:type", User)
], Profile.prototype, "user", void 0);
Profile = __decorate([
    Entity()
], Profile);
export default Profile;
//# sourceMappingURL=Profile.js.map
User.js
// User.js

var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
    var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d;
    if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);
    else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
    return c > 3 && r && Object.defineProperty(target, key, r), r;
};
var __metadata = (this && this.__metadata) || function (k, v) {
    if (typeof Reflect === "object" && typeof Reflect.metadata === "function") return Reflect.metadata(k, v);
};
import { Entity, OneToOne } from "typeorm";
import Profile from "./Profile.js";
let User = class User {
};
__decorate([
    OneToOne(() => Profile, profile => profile.user),
    __metadata("design:type", Profile)
], User.prototype, "profile", void 0);
User = __decorate([
    Entity()
], User);
export default User;
//# sourceMappingURL=User.js.map

And when we run the code we will get this error:

file:///Users/user/Documents/workspace/project/dist/entity/User.js:17
    __metadata("design:type", Profile)
                              ^

ReferenceError: Cannot access 'Profile' before initialization
    at file:///Users/user/Documents/workspace/project/dist/entity/User.js:17:31
    at ModuleJob.run (node:internal/modules/esm/module_job:195:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:331:24)
    at async Command.<anonymous> (file:///Users/user/Documents/workspace/project/dist/index.js:29:34)

The issue is that the statement __metadata("design:type", Profile) on User.js access Profile
while the statement __metadata("design:type", User) on Profile.js access User, thus creating a circular dependency between 2 files that require the other file to be already loaded in order to finish loading itself.

Since in these types of cases the metadata of the property type is redundant since the relation decorator already requires you to provide the type of the column (via () => Profile in this example), removing the property type information will not damage any functionality.

This is a known issue with tsconfig.json's emitDecoratorMetadata, fixing it on the TypeScript level is very complicated so I gave up on that.

*
* }
*/
export type Related<T> = T;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

not sure I like this name. Maybe call it Relation ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

@giladgd
Copy link
Contributor Author

giladgd commented Jan 25, 2022

@pleerock Please let me know if I can help to merge this PR

@pleerock
Copy link
Member

Thanks a lot for this contribution. Once I release a new version in couple days please make sure to check if both mjs and non-mjs commands are working properly in a production. Thank you!

@pleerock pleerock merged commit 3a694dd into typeorm:master Jan 31, 2022
@igolka97
Copy link

igolka97 commented Feb 4, 2022

Hello, @giladgd! Thanks for your job!

While I was looking for a solution to my ESM related problem, I came across this thread.

I've looked at all the changes and it still doesn't seem to solve my problem. I'm afraid to make a mistake, but here's my situation.

I'm using typeorm along with electron and quasar, which builds the entire project out of the box with webpack. In fact, my package.json does not and cannot contain the “module” item, but all my files work through ESM, and following the code, if I do not rename the files to mts, then it will still try to require, which will cause an error.

I will be very happy to help, I will try to deploy the situation locally, but I do not know how long it may take, because I may not have enough experience yet.

@KevinNovak
Copy link

@igolka97 Do you have compilerOptions.module set to "CommonJS" in your tsconfig by chance?

@loynoir
Copy link

loynoir commented Feb 4, 2022

Wow, awesome! Is there a release date having this?
Here is a .mts + esbuild-node-loader starter got stuck :(

@giladgd Is this PR compact with esbuild? Thanks.

@igolka97
Copy link

igolka97 commented Feb 4, 2022

@KevinNovak unfortunately yes, I have already tried. Still getting

import { Column, Entity, ManyToOne, PrimaryGeneratedColumn } from 'typeorm';
^^^^^^

SyntaxError: Cannot use import statement outside a module

I'm still investigating this issue and I think webpack is to blame.

@loynoir
Copy link

loynoir commented Feb 4, 2022

@igolka97 I think webpack is not very ESM friendly.

Have you try esbuild? It's fast and support ESM, and zero config.
Or rollup, one old ESM bundler.
Or maybe without bundler, using --loader hook, esbuild-node-loader

@giladgd
Copy link
Contributor Author

giladgd commented Feb 4, 2022

Hi @igolka97,

This PR was merged but a new version of TypeORM hasn't been released yet, so please wait until a new version is released (probably in a few days) before trying to use TypeORM in an ESM project.

Once the new version is released, you could run this command in order to generate a working ESM TS project to use as a starting point:

npx typeorm init --name MyProject --database postgres --module esm

@igolka97
Copy link

igolka97 commented Feb 4, 2022

@loynoir I using webpack since it using quasar bundler. Unfortunately this is payment for the convenience i get.

@giladgd
Copy link
Contributor Author

giladgd commented Feb 4, 2022

@loynoir I havn't tried using it with esbuild, but I don't see a reason why shouldn't it work, as long as esbuild supports importing node modules

@marcelgerber
Copy link

Great that we have this 🎉 ; I just realized that TypeORM is quite the stumbling stone when I attempted to convert our project to ES Modules.

Is there already a release date planned for the next version of TypeORM?

marcelgerber added a commit to owid/owid-grapher that referenced this pull request Feb 14, 2022
we're using a prerelase version of typeorm where ESM support (typeorm/typeorm#8536) is merged
marcelgerber added a commit to owid/owid-grapher that referenced this pull request Feb 14, 2022
we're using a prerelase version of typeorm where ESM support (typeorm/typeorm#8536) is merged
marcelgerber added a commit to owid/owid-grapher that referenced this pull request Feb 15, 2022
we're using a prerelase version of typeorm where ESM support (typeorm/typeorm#8536) is merged
marcelgerber added a commit to owid/owid-grapher that referenced this pull request Feb 16, 2022
we're using a prerelase version of typeorm where ESM support (typeorm/typeorm#8536) is merged
@pleerock
Copy link
Member

@marcelgerber it was released.

@marcelgerber
Copy link

@marcelgerber it was released.

Thank you, it's great!

@Zipper228
Copy link

I run into the error "ERR_LOADER_CHAIN_INCOMPLETE" when trying to run "npm start" on my ESM project with typeorm. Does anyone know how to fix it?

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