ESM support in cli

[zeakd]

Feature Description

The Problem

node 14 is now officially support ESM module, and ts-node is trying to support esm too.

https://nodejs.org/api/esm.html
TypeStrong/ts-node#1007

ESM module needs to set "type": "module" in package.json, but this option break typeorm cli because typeorm use require to import ts files

https://github.com/typeorm/typeorm/blob/master/src/connection/ConnectionOptionsReader.ts#L110
https://github.com/typeorm/typeorm/blob/master/src/util/DirectoryExportedClassesLoader.ts#L41

The Solution

use import() instead of require()
maybe package compiling environment needs to update.

Considered Alternatives

Additional Context

Relevant Database Driver(s)

• aurora-data-api
• aurora-data-api-pg
• better-sqlite3
• cockroachdb
• cordova
• expo
• mongodb
• mysql
• nativescript
• oracle
• postgres
• react-native
• sap
• sqlite
• sqlite-abstract
• sqljs
• sqlserver

Are you willing to resolve this issue by submitting a Pull Request?

• Yes, I have the time, and I know how to start.
• Yes, I have the time, but I don't know how to start. I would need guidance.
• No, I don't have the time, although I believe I could do it if I had the time...
• No, I don't have the time and I wouldn't even know how to start.

[nebkat]

All instances of require() could be replaced with a function in PlatformTools so that when the switch from require() to import() is made it is a single line change, or can be made version dependent. For now though we support all the way down to Node v10 so this change can't yet be made.

[cspotcode]

Here is how mocha supports both require() hooks and ESM hooks.
https://github.com/mochajs/mocha/blob/273dbbbf4c21debded604c2094591bdbcbed7705/lib/esm-utils.js#L11-L28

It's important for mocha to attempt require() first, in case there's a custom require.extensions hook installed. It detects a special node error and switches to import().

As far as I know, import() does not support require.extensions, so it's not true that you can use import() everywhere and get CommonJS support for free. You still need to attempt require().

Having said that, if you experiment and find out that I'm wrong, please let me know. Perhaps my information is out-of-date.

[zeakd]

@cspotcode It looks good. can typeorm use this fallback in PlatformTools?

[cspotcode]

Do you have more context? What is the goal?

…

On Tue, Dec 15, 2020, 8:22 AM Deokseong Kim ***@***.***> wrote: @cspotcode <https://github.com/cspotcode> It looks good. can typeorm use this fallback in PlatformTools? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#7159 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAC35OBKWKKVLLBB5XSWKXDSU5PKDANCNFSM4UOP5PIA> .

[cspotcode]

Disregard my last comment; I thought this was in a different issue.

Are you asking if typeorm can use the same technique as mocha? I think so yes. But I don't know anything about platformtools, so I don't know what it tries to do.

[zeakd]

@cspotcode oh right sorry. i thought you are in typeorm

I'll try mocha way and maybe can make pr next week or new year

[glen-84]

Here is how mocha supports both require() hooks and ESM hooks.
https://github.com/mochajs/mocha/blob/273dbbbf4c21debded604c2094591bdbcbed7705/lib/esm-utils.js#L11-L28

It's important for mocha to attempt require() first, in case there's a custom require.extensions hook installed. It detects a special node error and switches to import().

As far as I know, import() does not support require.extensions, so it's not true that you can use import() everywhere and get CommonJS support for free. You still need to attempt require().

Having said that, if you experiment and find out that I'm wrong, please let me know. Perhaps my information is out-of-date.

@cspotcode,

require.extensions has been deprecated since Node.js v0.10.6 – is it really necessary to support that?

I think you could just detect the package type and conditionally use import or require.

[cspotcode]

require.extensions is essentially the transformSource hook for CommonJS. It also affects resolution, so it's a little bit of the resolve hook too.

The age of its deprecation is brought up a lot. I understand why, but it's the only way to do certain things. If you want to do the equivalent of transformSource for CommonJS, you need to use require.extensions.

[glen-84]

@cspotcode,

I see. What type of libraries/projects are using transformSource / how common is its use?

Perhaps the best option would be to use import by default, and have an opt-out for those who may rely on the behaviour of require.extensions.

[cspotcode]

To get a sense for all the different require.extensions module loaders out there:
https://www.npmjs.com/package/interpret
The list above is extensive, so it will include some loaders that do not make much sense and some that do.

The big ones are things like ts-node, @babel/register, @swc/register, stuff like that.

[redplant3d]

Can you at least include a flag to configure this easily?

[GerkinDev]

typeorm/src/connection/ConnectionOptionsReader.ts

Line 111 in 749511d

const configModule = await require(configFile);

For this one, a dynamic import can be AFAICT a drop-in replacement, since it is already in an async context and auto-support of cjs is induced by one of the possible extensions. It would worth to add support to .mjs extension though once migrating to ESM.
In one of my programs, I use ts-node to load ts files on the fly imported via dynamic imports. I don't remember where did I saw the piece of doc talking about this behavior but this is possible in cjs (not sure for ESM interop though).

typeorm/src/util/DirectoryExportedClassesLoader.ts

Lines 36 to 41 in 749511d

	const dirs = allFiles
	.filter(file => {
	const dtsExtension = file.substring(file.length - 5, file.length);
	return formats.indexOf(PlatformTools.pathExtname(file)) !== -1 && dtsExtension !== ".d.ts";
	})
	.map(file => require(PlatformTools.pathResolve(file)));

This one is a bit trickier since this method is called in multiple places, some of them being in a sync context.

I think I'm gonna fork and try to tweak a bit.

[GerkinDev]

So, for what I've seen so far, there is some heavy tooling rework to do to make it happen. For instance, functional tests include ts files compiled as cjs to test cjs imports, so reworking tsconfig to build package as both esm & cjs would require some weird inclusion patterns matching those files. Or, other option, write them by hand as cjs/esm to avoid ts transpilation altogether.
Plus, mixing both esm & cjs is quite a pain with mocha.

I've bypassed the problem by modifying my build tool chains to use default imports and destructure them on the fly.

I somewhat doubt that the maintainers will accept easily the PR required to make it happen, since it would change a lot of things.

If a maintainer is passing by: have you tried anything about this ?

[Ginden]

About:

type Relation<T> = T;

I would suggest using propertyName: T | null in general for relations - it's possible that relation wasn't loaded.