feat: add TypeORM support #5801

hacksparrow · 2020-06-22T11:01:56Z

Taking the work in #4794 further, this PR adds TypeORM support in LB4. You can now use TypeORM entities and repositories in LB4.

The feature is implemented and working, for usage details refer to https://github.com/strongloop/loopback-next/blob/c7157f85cb8b05a43fdf59eabfd72892bf9a6662/packages/typeorm/README.md.

What's remaining?

CLEAN UP the repository
Write tests
Generate OpenAPI spec
Convert filters to TypeORM query (everything not required in the first version, just need to develop a functional high-level idea about how to go about it)
Add documentation to website

Checklist

👉 Read and sign the CLA (Contributor License Agreement) 👈

npm test passes on your machine
New tests added or existing tests modified to cover all changes
Code conforms with the style guide
API Documentation in code was updated
Documentation in /docs/site was updated
Affected artifact templates in packages/cli were updated
Affected example projects in examples/* were updated

👉 Check out how to submit a PR 👈

achrinza

Did a quick skim and noticed some minor issues.

Also, should we have a booter for TypeORM entities, similar to the ones for lb4 models?

Other than that, everything else LGTM.

achrinza · 2020-06-22T11:31:24Z

packages/typeorm/LICENSE

@@ -0,0 +1,25 @@
+Copyright (c) IBM Corp. 2017,2019.


Inconsistent copyright.

Ditto for the other copyright headers.

achrinza · 2020-06-22T11:32:48Z

packages/typeorm/LICENSE

@@ -0,0 +1,25 @@
+Copyright (c) IBM Corp. 2017,2019.
+Node module: @loopback/repository


Inconsistent node module name header.

Ditto for the other module name headers.

Haha, yea need to clean up.

hacksparrow · 2020-06-22T12:59:36Z

Also, should we have a booter for TypeORM entities, similar to the ones for lb4 models?

Yes, there's a booter - https://github.com/strongloop/loopback-next/pull/5801/files#diff-2a23c8ed814e1a88b05bcacbeec6689c.

raymondfeng · 2020-06-22T15:30:48Z

tsconfig.json

+      "path": "packages/typeorm/tsconfig.json"
+    },
+    {
+      "path": "sandbox/appa/tsconfig.json"


Please rm -rf sandbox/appa.

Yea, need to clean up.

raymondfeng · 2020-06-22T15:32:08Z

extensions/typeorm/src/typeorm.component.ts

+//   Server,
+//   Subscription,
+// } from '@loopback/core';
+


Please remove the commented out code.

raymondfeng · 2020-06-22T15:36:03Z

packages/typeorm/src/decorators/typeorm.decorator.ts

+  export function connection(connectionName?: string) {
+    return inject(
+      '',
+      {decorator: '@typeormConnection'},


We typically use typeorm.connection as the name.

Indeed. That's a remnant of something I was trying, will clean it up.

bajtos · 2020-06-23T11:50:37Z

packages/typeorm/README.md

+  constructor(options: ApplicationConfig = {}) {
+    super(options);
+    ...
+    this.component(TypeOrmComponent);


I am proposing to modify TypeOrmMixin to call this.component(TypeOrmComponent) automatically, to get the following user experience:

As a LB4 app developer wanting to add TypeORM to my app, I need to change a single place in my app file only.

To be honest, I think it should be possible to design this extension to provide the app Mixin only, it should not be necessary to have a Component class. See how @loopback/repository is implemented - it does not have any Component class either.

bajtos · 2020-06-23T11:51:41Z

packages/typeorm/src/decorators/typeorm.decorator.ts

+        injection: Readonly<Injection>,
+        session: ResolutionSession,
+      ) => {
+        return getConnection(ctx, connectionName);


IIUC, this creates "virtual" bindings that can be injected but that are not tracked by Context and therefore not visible in context-inspector UI.

Can we find a way how to create regular bindings for individual connections please?

The same comment applies to other inject-like decorators below.

IIUC, this creates "virtual" bindings that can be injected but that are not tracked by Context and therefore not visible in context-inspector UI.

Can we find a way how to create regular bindings for individual connections please?

☝️ Please make sure this comment is either addressed as part of this original PR or there is a follow-up issue to fix binding inspection ASAP.

Note to self: Create follow up task.

bajtos · 2020-06-23T11:58:45Z

packages/typeorm/src/mixins/typeorm.mixin.ts

+  superClass: T,
+) {
+  return class extends superClass {
+    typeormConnectionOptions: ConnectionOptions[] = [];


As we discussed in the call, I find the design where we keep an index-ordered list of connection options as suboptimal. Instead, I am proposing to store TypeORM's connection manager instance on the application object and create a method like app.connection to register new connections with the app. The API should also allow the user to give each connection a name.

// in the mixin return class extends superClass { connectionManager: ConnectionManager; constructor(...args: any[]) { super(args); this.connectionManager = new ConnectionManager(); } connection(connectionConfig: ConnectionOptions) { const connection = this.connectionManager.create(connectionConfig); const name = connection.name; this.bind(`connections.${name}`).toDynamicValue(() => this.connectionManager.getConnection(name)); } }

A use case to consider:

It should be possible to add new connections (and remove existing ones) at application runtime, after the app has booted & started. See How to build models, repositories and controllers dynamically at runtime #4296.

As we discussed in the call, I find the design where we keep an index-ordered list of connection options as suboptimal. Instead, I am proposing to store TypeORM's connection manager instance on the application object and create a method like app.connection to register new connections with the app. The API should also allow the user to give each connection a name.

❗ IMO, this comment must be resolved before this pull request can be landed.

bajtos · 2020-06-23T12:01:13Z

packages/typeorm/src/types.ts

+  ExpoConnectionOptions,
+  ExpoDriver,
+  ExpoQueryRunner,
+};


Do we really need that long list of exports 👆 and the long list of imports 👇 ? Isn't it enough to export * from 'typeorm';?

For some reason they seem to have decided to export only these from ./driver.

export {Driver} from "./driver/Driver"; export * from "./driver/mongodb/typings"; export * from "./driver/sqlserver/MssqlParameter"; export {ColumnType} from "./driver/types/ColumnTypes"; export * from "./driver/types/DatabaseType";

So we have to do the importing and exporting for our users.

Fair enough 🤷

bajtos · 2020-06-23T12:02:40Z

packages/typeorm/src/utils.ts

+ * @param entityCtor - The entity constructor (e.g. `Product`)
+ * @param options - Additional options
+ */
+export function getModelSchemaRef<T extends object>(


As discussed in the call, I am proposing to drop Ref part and emit regular JSON schema. The ref-based schemas were needed before OpenAPI schema consolidation was introduced. I believe with consolidation in place, we no longer need to worry about avoiding duplicated schema definitions in controller methods.

export function modelToJsonSchema<T extends object>( ctor: Function & {prototype: T}, jsonSchemaOptions: JsonSchemaOptions<T> = {}, ): JsonSchema { const allColumns: ColumnMetadataArgs[]= getMetadataArgsStorage().columns; const modelColumns = allColumns.filter(c => c.target === modelCtor); const schema: JsonSchema = { // set title (to modelCtor.modelName?), etc. properties: {} }; for (c of modelColumns) { schema.properties[c.propertyName] = { type: c.options.type, } } return schema; } // getJsonSchema(modelCtor) --> call modelToJsonSchema under the hood + cache the result // getModelSchema(modelCtor) --> convert getJsonSchema() to OpenApiSpec format

As discussed in the call, I am proposing to drop Ref part and emit regular JSON schema. The ref-based schemas were needed before OpenAPI schema consolidation was introduced. I believe with consolidation in place, we no longer need to worry about avoiding duplicated schema definitions in controller methods.

❗I'd like you to address my comment before the PR is landed.

If it's too much work to get even a minimal conversion working, then I am proposing the following:

rename getModelSchemaRef to getModelSchema

change the return type from SchemaRef to SchemaObject

change the implementation to return a "dummy" schema describing any object value

export function getModelSchemaRef<T extends object>( entityCtor: Function & {prototype: T}, options?: JsonSchemaOptions<T>, ): SchemaRef { // TODO: generate proper OpenAPI schema describing entity properties, etc. return { title: entityCtor.name, type: 'object' }; }

Personally, I'll try to get rid of SchemaRef (and thus the dependency on @loopback/rest) by removing the description of the return type or using object instead. For the first iteration, we don't need options either, since they are not supported anyways.

export function getModelSchemaRef<T extends object>( entityCtor: Function & {prototype: T}, ) { return { // ... }; }

bajtos · 2020-06-23T12:05:41Z

Convert filters to TypeORM query (everything not required in the first version, just need to develop a functional high-level idea about how to go about it)

Posting few ideas from slack chat:

Juggler implementation starts here: https://github.com/strongloop/loopback-connector/blob/7d43b461d13e2e67fd07cee4ae22c1e19d0ea848/lib/sql.js#L1332-L1362

For TypeORM, we want something like this:

{where: {name: 'miroslav'}, limit: 10} => createQueryBuilder().where('user.name = :name', {name: 'miroslav'}).limit(10);

IMO, TypeORM users should not be forced to require @loopback/repository to get the Filter object (and also getFilterSchemaFor). In the spike, it may be ok to copy the relevant type descriptions & schema builder from @loopback/repository to your new extension. After the spike, I think we should extract Filter, getFilterSchemaFor and other related code into a new shared package, e.g. @loopback/filter.

In LB3, we created loopback-filters package with a similar intention, but it was half-baked and quickly got out of sync with the main filtering logic implemented in the memory connector. So I think it's a valid move to start building a shared filtering package again, and get it right this time :)

raymondfeng · 2020-06-24T16:42:20Z

packages/boot/src/booters/typeorm-connection.booter.ts

+    super(
+      projectRoot,
+      // Set TypeORM connection options if passed in via bootConfig
+      Object.assign({}, ConnectionDefaults, entityConfig),


This is wrong - all properties are merged into the 1st arg and we won't be able to access it afterward.

I believe Object.assign returns the 1st argument and therefore this line works as expected

$ node > console.log(Object.assign({}, {foo: 'bar'})) { foo: 'bar' } undefined

raymondfeng · 2020-06-24T16:45:03Z

packages/typeorm/src/mixins/typeorm.mixin.ts

+/**
+ * Objects with open properties
+ */
+export interface AnyObject {


Let's use Record<string, any> instead.

raymondfeng · 2020-06-24T16:47:00Z

packages/boot/src/booters/typeorm-connection.booter.ts

+      }
+    }
+  }
+}


I don't see test coverage for the booter.

raymondfeng · 2020-06-24T16:47:33Z

packages/boot/src/booters/typeorm-connection.booter.ts

+  async load() {
+    for (const file of this.discovered) {
+      const exported = require(file);
+      const connectionKey = Object.keys(exported)[0];


This is not clear to me how a connection artifact look like.

+1

I think the actual config object should have ConnectionOptions type, see https://github.com/typeorm/typeorm/blob/master/src/connection/ConnectionOptions.ts and https://github.com/typeorm/typeorm/blob/fc7a4f13f6270c9d5206358ebd78adb0ca690b3d/src/index.ts#L127

Suggested change

const connectionKey = Object.keys(exported)[0];

const connectionKey: ConnectionOptions = Object.keys(exported)[0];

IIUC, you are trying to register the first exported thing as the connection object. I find this design as providing poor user experience. If the file happens to export more than one config object, then we should either load all of them, or tell the user what has been ignored.

Please take a look at how existing booters are dealing with this edge case and implement the same behavior in your booter for consistency.

bajtos

I am afraid most of my comments are still not addressed. Unfortunately, it's not clear what is the scope of your work (and this pull request), therefore it's difficult to draw the line between things that must be resolved now and improvements that can be made later.

IMO, it's important to address the following:

Add few basic tests to ensure the code coverage is not dropping too much.
Change getModelSchemaRef to getModelSchema
Add a list of known limitations to README
Redesign how connections are registered in the app (remove an array of connection options, introduce app.connection() method).
Get rid of TypeOrmConnections or at least rename it to a singular form.
Either remove all coupling with REST/OpenAPI, or at least depend on @loopback/openapi-v3 instead of @loopback/rest

bajtos · 2020-06-25T11:57:29Z

packages/boot/src/booters/typeorm-connection.booter.ts

+  async load() {
+    for (const file of this.discovered) {
+      const exported = require(file);
+      const connectionKey = Object.keys(exported)[0];


+1

I think the actual config object should have ConnectionOptions type, see https://github.com/typeorm/typeorm/blob/master/src/connection/ConnectionOptions.ts and https://github.com/typeorm/typeorm/blob/fc7a4f13f6270c9d5206358ebd78adb0ca690b3d/src/index.ts#L127

Suggested change

const connectionKey = Object.keys(exported)[0];

const connectionKey: ConnectionOptions = Object.keys(exported)[0];

IIUC, you are trying to register the first exported thing as the connection object. I find this design as providing poor user experience. If the file happens to export more than one config object, then we should either load all of them, or tell the user what has been ignored.

Please take a look at how existing booters are dealing with this edge case and implement the same behavior in your booter for consistency.

bajtos · 2020-06-25T12:00:14Z

packages/typeorm/README.md

+npm install --save @loopback/typeorm
+```
+
+## Usage


Suggested change

## Usage

## Basic use

See https://loopback.io/doc/en/contrib/README-guidelines.html

packages/typeorm/README.md

bajtos · 2020-06-25T12:09:31Z

packages/typeorm/README.md

+```ts
+// src/controllers/book.controller.ts
+import {get, post, Request, requestBody} from '@loopback/rest';
+import {getModelSchemaRef, Repository, typeorm} from '@loopback/typeorm';


As discussed before, this should be just getModelSchema. (No need to use $ref schemas anymore, OpenAPI consolidation will take care of de-duplicating schemas.)

IMO, @loopback/typeorm should not be coupled with OpenAPI, I think we should either tell users to apply jsonToSchemaObject on the schema produced by @loopback/typeorm, or else introduce a new package for things that are specific to TypeORM+OpenAPI.

IMO, @loopback/typeorm should not be coupled with OpenAPI,

Need elaboration.

@loopback/openapi-v3 provides things like @get and @api decorators.

As a user building gRPC or Pub/Sub application using @loopback/typeorm for persistence, why do I have to install dependencies for OpenAPI, when I am not using any of that?

Also if we add gRPC support in the future and implement code to generate gRPC schemas (protocol buffer message type definitions) from TypeORM entities, are we going to add gRPC to @loopback/typeorm dependencies and thus force REST/OpenAPI users to install gRPC they are not using?

Anyhow, I think this is out of scope of this pull request, but please do create a follow-up task to continue this discussion and rework the code accordingly.

Note to self: Create follow up task.

bajtos · 2020-06-25T12:11:06Z

packages/typeorm/README.md

+    const book = new Book();
+    book.title = data.title;
+    book.published = false;
+    return await this.bookRepo.save(book);


Can we use repository.create API?

this.bookRepo.create(data);

See https://typeorm.io/#/repository-api

The object is not being saved for some reason; don't want to troubleshoot right now, let's handle the highest priority items first. Will give a try again later today, if not, will open a follow up task.

Note to self: Create follow up task.

Perhaps add a code comment to this code snippet, to explain the problem to our readers too?

packages/typeorm/src/services/connections.ts

bajtos · 2020-06-25T12:31:02Z

packages/typeorm/src/utils.ts

+// This file is licensed under the MIT License.
+// License text available at https://opensource.org/licenses/MIT
+
+import {JsonSchemaOptions, SchemaRef} from '@loopback/rest';


❗ @loopback/typeorm should not require consumers to use @loopback/rest.

At minimum, change this line to import OpenAPI types from @loopback/openapi-v3.

As I mentioned above, I think it would be best to move OpenAPI-related parts into a new package, but I guess that can be done in a follow-up pull request.

Note to self: Create follow up task.

bajtos · 2020-06-25T12:31:44Z

packages/typeorm/src/utils.ts

+ * @param entityCtor - The entity constructor (e.g. `Product`)
+ * @param options - Additional options
+ */
+export function getModelSchemaRef<T extends object>(


As discussed in the call, I am proposing to drop Ref part and emit regular JSON schema. The ref-based schemas were needed before OpenAPI schema consolidation was introduced. I believe with consolidation in place, we no longer need to worry about avoiding duplicated schema definitions in controller methods.

❗I'd like you to address my comment before the PR is landed.

bajtos · 2020-06-25T12:39:50Z

packages/typeorm/src/utils.ts

+ * @param entityCtor - The entity constructor (e.g. `Product`)
+ * @param options - Additional options
+ */
+export function getModelSchemaRef<T extends object>(


If it's too much work to get even a minimal conversion working, then I am proposing the following:

rename getModelSchemaRef to getModelSchema

change the return type from SchemaRef to SchemaObject

change the implementation to return a "dummy" schema describing any object value

export function getModelSchemaRef<T extends object>( entityCtor: Function & {prototype: T}, options?: JsonSchemaOptions<T>, ): SchemaRef { // TODO: generate proper OpenAPI schema describing entity properties, etc. return { title: entityCtor.name, type: 'object' }; }

Personally, I'll try to get rid of SchemaRef (and thus the dependency on @loopback/rest) by removing the description of the return type or using object instead. For the first iteration, we don't need options either, since they are not supported anyways.

export function getModelSchemaRef<T extends object>( entityCtor: Function & {prototype: T}, ) { return { // ... }; }

bajtos

Thank you for addressing my comments, @hacksparrow, I like the new version sooo much more! 😍

The pull request looks mostly good now, I left few non-blocking comments below.

Please work with @raymondfeng to decide which of my suggestions are ok to leave out and make sure to get his approval before landing the PR.

packages/boot/src/booters/typeorm-connection.booter.ts

bajtos · 2020-06-26T08:46:35Z

packages/typeorm/README.md

  constructor(options: ApplicationConfig = {}) {
    super(options);
    ...
-    this.component(TypeOrmComponent);
-    ...
  }


Nitpick: we no longer need to show the constructor.

export class AppaApplication extends BootMixin( ServiceMixin(TypeOrmMixin(RestApplication)), ) { // ... }

bajtos · 2020-06-26T08:47:57Z

packages/typeorm/README.md

@@ -1,6 +1,7 @@
 # @loopback/typeorm

-This module enables TypeORM support in LoopBack.
+This module enables TypeORM support in LoopBack. For pending features, refer to
+the "Pending" section.


Suggested change

the "Pending" section.

the "Limitations" section.

(To match the real section name.)

Even better:

Suggested change

the "Pending" section.

the [Limitations](#limitations) section below.

(Just make sure the link works this way, I am not sure what's the exact anchor URL.)

bajtos · 2020-06-26T08:48:17Z

packages/typeorm/README.md

@@ -12,25 +13,22 @@ This module enables TypeORM support in LoopBack.
 npm install --save @loopback/typeorm
 ```

-## Usage
+## Basic Usage


Per our README guidelines:

Suggested change

## Basic Usage

## Basic Use

bajtos · 2020-06-26T08:49:41Z

packages/typeorm/README.md

+
+1. Database migration
+2. Custom repository
+3. Query filters


Let's clarify this item. I believe it is already possible to use TypeORM filters, right? IIUC, we are missing support for LoopBack-style filters and exposing filters via REST API.

bajtos · 2020-06-26T08:52:04Z

packages/typeorm/src/decorators.ts

@@ -62,6 +62,6 @@ export namespace typeorm {
 * @param connectionName - Optional connection name
 */
 async function getConnection(ctx: Context, connectionName?: string) {
-  const connections = await ctx.get(TypeOrmBindings.CONNECTIONS);
-  return connections.manager.get(connectionName);
+  const manager = await ctx.get(TypeOrmBindings.MANAGER);


I expect this method to be called on hot paths, can we make this method sync to improve performance?

function getConnection(ctx: Context, connectionName?: string): Connection { const manager = ctx.getSync(TypeOrmBindings.MANAGER); return manager.get(connectionName); }

Feel free to leave this improvement out of scope of this pull request.

Note to self: Create follow up task.

bajtos · 2020-06-26T08:55:11Z

packages/typeorm/src/mixin.ts

+@lifeCycleObserver('datasource', {
+  scope: BindingScope.SINGLETON,
+})
+export class StartStop implements LifeCycleObserver {


Please mention "typeorm" in the class name, to distinguish this class from other similar observers.

For example:

Suggested change

export class StartStop implements LifeCycleObserver {

export class StartStopTypeOrmConnections implements LifeCycleObserver {

I believe we use nouns in class names, perhaps something like TypeOrmLifeCycleManager would be a better name? @raymondfeng do you have any suggestions?

Using TypeOrmLifeCycleManager.

bajtos · 2020-06-26T08:56:13Z

packages/typeorm/src/utils.ts

+import {ColumnType} from 'typeorm/driver/types/ColumnTypes';
+import {ColumnMetadataArgs} from 'typeorm/metadata-args/ColumnMetadataArgs';
+
+const ModelSchemaCache = new WeakMap();


Nitpick: we use camelCase for variable names (CamelCase is for type/class names).

Suggested change

const ModelSchemaCache = new WeakMap();

const modelSchemaCache = new WeakMap();

bajtos · 2020-06-26T08:57:01Z

packages/typeorm/src/utils.ts

+  const cached = ModelSchemaCache.get(modelCtor);
+  if (cached) {
+    return cached;
+  } else {


Please remove the else block to reduce the indentation level.

if (cached) { return cached; } const allColumns: ColumnMetadataArgs[] = getMetadataArgsStorage().columns; // ...

bajtos · 2020-06-26T09:00:13Z

packages/typeorm/src/utils.ts

+    return 'boolean';
+  } else {
+    throw new Error(
+      `Type conversion of ${func} to OpenAPI format not implemented.`,


UX improvement:

Please include the entity name and the property name in the error message.

Discussion point:

In your proposal, applications cannot use unsupported types at all. Isn't that too limiting, considering that those types works at ORM layer and we just don't know how to describe them in OpenAPI? How about changing this line to print a warning and return a value saying the property does not have any type constraints? (I think undefined should work.)

Done.

} else { debug( `${entity}.${property}: Type conversion of ${func} to OpenAPI format not implemented.`, ); return undefined; }

raymondfeng · 2020-07-07T16:38:47Z

docs/site/Using-TypeORM-with-LoopBack.md

+title: 'How to use TypeORM with LoopBack'
+keywords: LoopBack 4.0, LoopBack 4, TypeORM
+sidebar: lb4_sidebar
+permalink: /doc/en/lb4/Using-TypeORM-with-LoopBack.html


Let's use Using-typeorm-with-loopback.html to be consistent with other links.

raymondfeng · 2020-07-07T16:39:28Z

docs/site/Using-TypeORM-with-LoopBack.md

+
+## Overview
+
+If you would like to use TypeORM with LoopBack, it is possible with the use of


Let's introduce TypeORM and have a link here.

raymondfeng · 2020-07-07T16:40:24Z

docs/site/Using-TypeORM-with-LoopBack.md

+}
+```
+
+### Creating Connections


Maybe it's better to have Creating Connections come before Creating Entities?

raymondfeng · 2020-07-07T16:41:09Z

docs/site/Using-TypeORM-with-LoopBack.md

+[TypeORM documentation](https://github.com/typeorm/typeorm#installation) for the
+supported databases and the underlying drivers.
+
+### Creating controllers


Creating Controllers

raymondfeng · 2020-07-07T16:42:36Z

docs/site/sidebars/lb4_sidebar.yml

+    - title: 'How to use TypeORM with LoopBack'
+      url: Using-TypeORM-with-LoopBack.html
+      output: 'web, pdf'
+


Can we move it to be after Accessing Databases?

raymondfeng · 2020-07-07T16:43:50Z

packages/typeorm/README.md

+
+## Overview
+
+**NOTE**: This module is experimental and evolving.


Let's use the same disclaimer as https://github.com/strongloop/loopback-next/tree/master/extensions/cron#stability-%EF%B8%8Fexperimental%EF%B8%8F

raymondfeng · 2020-07-07T16:45:14Z

packages/typeorm/package.json

+  "main": "dist/index.js",
+  "types": "dist/index.d.ts",
+  "engines": {
+    "node": ">=10"


raymondfeng · 2020-07-07T16:47:23Z

packages/typeorm/package.json

+    "@loopback/rest": "^5.2.0",
+    "@loopback/service-proxy": "2.3.4",
+    "@loopback/testlab": "^3.1.7",
+    "@types/json-schema": "^7.0.5",


Let's use the following commands to update its deps on other LB4 packages:

cd packages/typeorm lb4 update --yes --force --skip-install cd ../.. node bin/rebuild-package-locks.js @loopback/typeorm

raymondfeng · 2020-07-07T16:47:56Z

packages/typeorm/package.json

@@ -0,0 +1,53 @@
+{


Maybe we should move it to extensions/typeorm?

raymondfeng · 2020-07-07T16:48:41Z

packages/typeorm/package.json

+    "access": "public"
+  },
+  "devDependencies": {
+    "@loopback/boot": "^2.3.5",


This should be a regular dependency.

raymondfeng · 2020-07-07T16:49:04Z

packages/typeorm/package.json

+  },
+  "dependencies": {
+    "@loopback/core": "^2.4.1",
+    "@loopback/openapi-v3": "3.4.4",


Do we need it as a regular dependency?

Yes, in https://github.com/strongloop/loopback-next/blob/typeorm-yaapa/packages/typeorm/src/typeorm.utils.ts.

raymondfeng · 2020-07-07T16:50:32Z

packages/typeorm/src/typeorm.booter.ts

+ * Default ArtifactOptions for TypeORM connection.
+ */
+export const ConnectionDefaults: ArtifactOptions = {
+  dirs: ['connections'],


Let's prefix the directory, such as typeorm-connections.

raymondfeng · 2020-07-07T16:53:20Z

packages/typeorm/src/__tests__/fixtures/connections/sqlite.connection.ts

+  name: 'my-db',
+  type: 'sqlite',
+  database: path.join(__dirname, './mydb.sql'),
+  entities: [Book],


I think it makes sense to have a booter for TypeORM entities too.

That's the TypeORM convention, I think we should leave it alone. We just provide the glue, everything else - Entities, Connections, Respository methods - remain 100% pure TypeORM.

dhmlau · 2020-07-08T13:45:23Z

extensions/typeorm/README.md

+   (e.g. to implement bookRepo.findByTitle(title)).
+6. [Database migration](https://github.com/strongloop/loopback-next/issues/5898)
+
+We will be implementing them progressively.


We might want to make it as "Community contribution is welcome".

dhmlau · 2020-07-08T13:47:20Z

docs/site/Using-TypeORM-with-LoopBack.md

+permalink: /doc/en/lb4/Using-typeorm-with-loopback.html
+---
+
+## Overview


Perhaps we can use the README.md content directly instead of duplicating the content?
See the JWT authentication extension docs as an example.

dhmlau · 2020-07-08T13:48:41Z

docs/site/sidebars/lb4_sidebar.yml

@@ -242,6 +242,13 @@ children:
      url: Database-migrations.html
      output: 'web, pdf'

+  - title: 'Using TypeORM'


any reason we need a parent item instead of having "How to use TypeORM with LoopBack" as the top level?

There will be more articles under it later. Eg: "How to create custom repositories", etc.

raymondfeng · 2020-07-08T15:46:19Z

extensions/typeorm/package.json

+  "repository": {
+    "type": "git",
+    "url": "https://github.com/strongloop/loopback-next.git",
+    "directory": "packages/typeorm"


extensions/typeorm

raymondfeng · 2020-07-08T15:46:44Z

extensions/typeorm/src/__tests__/fixtures/application.ts

@@ -0,0 +1,16 @@
+// Copyright IBM Corp. 2019. All Rights Reserved.
+// Node module: @loopback/boot


Please fix license header

You can use lb4 copyright command under extensions/typeorm.

raymondfeng · 2020-07-08T15:47:38Z

extensions/typeorm/src/__tests__/fixtures/controllers/book.controller.ts

@@ -0,0 +1,59 @@
+// Copyright IBM Corp. 2020. All Rights Reserved.
+// Node module: @loopback/rest


raymondfeng · 2020-07-08T15:48:32Z

extensions/typeorm/src/__tests__/fixtures/entities/book.entity.ts

@@ -0,0 +1,18 @@
+// Copyright IBM Corp. 2020. All Rights Reserved.


entities -> typeorm-entities?

raymondfeng · 2020-07-08T15:49:19Z

extensions/typeorm/src/typeorm.booter.ts

@@ -0,0 +1,73 @@
+// Copyright IBM Corp. 2020. All Rights Reserved.
+// Node module: @loopback/graphql


raymondfeng · 2020-07-08T15:50:12Z

extensions/typeorm/src/typeorm.booter.ts

@@ -0,0 +1,73 @@
+// Copyright IBM Corp. 2020. All Rights Reserved.


typeorm-connection.booter.ts?

hacksparrow mentioned this pull request Jun 22, 2020

Integration with TypeORM #4853

Closed

3 tasks

hacksparrow force-pushed the typeorm-yaapa branch 2 times, most recently from af3d788 to 82385ba Compare June 22, 2020 11:11

achrinza reviewed Jun 22, 2020

View reviewed changes

hacksparrow force-pushed the typeorm-yaapa branch from 82385ba to c7157f8 Compare June 22, 2020 13:10

raymondfeng reviewed Jun 22, 2020

View reviewed changes

bajtos reviewed Jun 23, 2020

View reviewed changes

hacksparrow force-pushed the typeorm-yaapa branch from c7157f8 to f77bd30 Compare June 24, 2020 15:11

raymondfeng requested changes Jun 24, 2020

View reviewed changes

bajtos suggested changes Jun 25, 2020

View reviewed changes

hacksparrow force-pushed the typeorm-yaapa branch from 8231130 to 09faf13 Compare June 26, 2020 07:24

bajtos approved these changes Jun 26, 2020

View reviewed changes

hacksparrow force-pushed the typeorm-yaapa branch from 09faf13 to d714638 Compare June 26, 2020 09:41

bajtos mentioned this pull request Jun 26, 2020

[RFC WIP] feat(typeorm): add a component for TypeORM integration #4794

Closed

7 tasks

hacksparrow force-pushed the typeorm-yaapa branch 2 times, most recently from beb8d98 to 23bb5f4 Compare June 26, 2020 10:33

bajtos mentioned this pull request Jun 26, 2020

feat: mongoose extension #5201

Closed

7 tasks

hacksparrow force-pushed the typeorm-yaapa branch 2 times, most recently from e178db3 to 150dd4a Compare July 7, 2020 15:39

hacksparrow marked this pull request as ready for review July 7, 2020 15:39

hacksparrow requested a review from jannyHou as a code owner July 7, 2020 15:39

raymondfeng reviewed Jul 7, 2020

View reviewed changes

packages/typeorm/package.json Outdated

@@ -0,0 +1,53 @@

{

Copy link

Contributor

raymondfeng Jul 7, 2020

Choose a reason for hiding this comment

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

Maybe we should move it to extensions/typeorm?

raymondfeng reviewed Jul 7, 2020

View reviewed changes

hacksparrow force-pushed the typeorm-yaapa branch 2 times, most recently from dc00546 to 3e6edaf Compare July 8, 2020 13:21

dhmlau reviewed Jul 8, 2020

View reviewed changes

hacksparrow force-pushed the typeorm-yaapa branch from a7c513f to 3cc7cac Compare July 8, 2020 13:59

raymondfeng changed the title ~~[WIP] TypeORM support~~ feat: add TypeORM support Jul 8, 2020

raymondfeng reviewed Jul 8, 2020

View reviewed changes

dhmlau assigned hacksparrow Jul 8, 2020

dhmlau added the feature label Jul 8, 2020

dhmlau added this to the Jul 2020 milestone Jul 8, 2020

hacksparrow force-pushed the typeorm-yaapa branch 2 times, most recently from 6b611bd to 182ee62 Compare July 8, 2020 16:34

feat(typeorm): add a component for TypeORM integration

61a8c54

hacksparrow force-pushed the typeorm-yaapa branch from f8e75e8 to 61a8c54 Compare July 8, 2020 19:38

raymondfeng approved these changes Jul 8, 2020

View reviewed changes

hacksparrow merged commit b4e984a into master Jul 8, 2020

hacksparrow deleted the typeorm-yaapa branch July 8, 2020 20:13

		@@ -0,0 +1,25 @@
		Copyright (c) IBM Corp. 2017,2019.
		Node module: @loopback/repository

	const connectionKey = Object.keys(exported)[0];
	const connectionKey: ConnectionOptions = Object.keys(exported)[0];

	the "Pending" section.
	the [Limitations](#limitations) section below.

	export class StartStop implements LifeCycleObserver {
	export class StartStopTypeOrmConnections implements LifeCycleObserver {

	const ModelSchemaCache = new WeakMap();
	const modelSchemaCache = new WeakMap();


		## Overview

		If you would like to use TypeORM with LoopBack, it is possible with the use of


		## Overview

		NOTE: This module is experimental and evolving.

		@@ -0,0 +1,16 @@
		// Copyright IBM Corp. 2019. All Rights Reserved.
		// Node module: @loopback/boot

		@@ -0,0 +1,59 @@
		// Copyright IBM Corp. 2020. All Rights Reserved.
		// Node module: @loopback/rest

		@@ -0,0 +1,18 @@
		// Copyright IBM Corp. 2020. All Rights Reserved.

		@@ -0,0 +1,73 @@
		// Copyright IBM Corp. 2020. All Rights Reserved.
		// Node module: @loopback/graphql

feat: add TypeORM support #5801

feat: add TypeORM support #5801

Conversation

hacksparrow commented Jun 22, 2020 • edited Loading

Checklist

achrinza left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hacksparrow commented Jun 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bajtos commented Jun 23, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bajtos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bajtos left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hacksparrow commented Jun 22, 2020 •

edited

Loading