0.3.0: Naming strategy ignored by migration:generate

[haggholm]

Issue Description

I upgraded TypeORM from 0.2.41 to 0.3.0. I made the necessary changes to get the project building against the updated APIs, and modified my scripts to generate migrations to use the new data source specification rather than the old ormconfig. In particular, when it comes to the configuration for CLI commands, I basically changed ormconfig.js module.exports = { type: 'postgres', ... } to exports.postgres = { type: 'postgres', ... }, and everything seemed to work.

…Except that the migration I generate drops every single constraint and index in my schema and recreates them according to the default naming strategy. It does not appear that the schema builder calls the custom naming strategy at all. From the changelog and documentation, it does not appear to me that the naming strategy options should have changed.

Expected Behavior

When generating migrations, it should call our custom naming strategy and generate constraint names like dossiers_tenantId_fkey.

Actual Behavior

All the indices, constraints, etc. are renamed to the TypeORM default strategy with uninformative names like FK_3ce1dd1736f8caad49f49.

Steps to Reproduce

I do not currently have a minimal repro example, sorry.

My Environment

Dependency	Version
Operating System	Linux (Ubuntu/PopOS 21.10)
Node.js version	16.14.0
Typescript version	4.6.2
TypeORM version	0.3.0

Additional Context

Relevant Database Driver(s)

I have only tested this in Postgres.

DB Type	Reproducible
aurora-mysql	no
aurora-postgres	no
better-sqlite3	no
cockroachdb	no
cordova	no
expo	no
mongodb	no
mysql	no
nativescript	no
oracle	no
postgres	yes
react-native	no
sap	no
sqlite	no
sqlite-abstract	no
sqljs	no
sqlserver	no

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, but I can support (using donations) development.
• ✅ No, I don’t have the time and I’m okay to wait for the community / maintainers to resolve this issue.

[pleerock]

new CLI commands were not documented yet, but for some of the commands like migration:generate you can pass "dataSource" option to specify where your database source is and it must use it's naming strategy.

migration:generate -d ./src/my-data-source.ts`

Please confirm if you run this commands and specify data source.

[haggholm]

new CLI commands were not documented yet, but for some of the commands like migration:generate you can pass "dataSource" option to specify where your database source is and it must use it's naming strategy.

I did specify the datasource, yes. That’s what I meant in the original issue description: I took an existing ormconfig.js for 0.2.41, changed the export, and otherwise left it unchanged, then passed this as the --dataSource parameter to the CLI tool.

So: In the ormconfig.js that worked for 0.2.41, there’s a { ..., namingStrategy: new MyNamingStrategy(), ... }, and that part is unchanged (except that it’s now part of a named export); and the typeorm CLI tool is certainly using this --dataSource configuration (else it wouldn’t have DB credentials, after all). But migration:generate never calls any methods on MyNamingStrategy. In fact, it behaves the same whether I pass { namingStrategy: new MyNamingStrategy() } or { namingStrategy: 5 }; it clearly never attempts any method calls.

So, basically,

// 0.2.41 ormconfig.js: connection info and namingStrategy are used
module.exports = {
  type: 'postgres',
  schema: 'public',
  host: "localhost",
  username: "postgres",
  password: "...",
  database: "...",
  namingStrategy: new MyNamingStrategy(),
  ...
};

// 0.3.0 datasource.js: connection info is used, but namingStrategy is ignored
exports.postgres = new DataSource({
  type: 'postgres',
  schema: 'public',
  host: "localhost",
  username: "postgres",
  password: "...",
  database: "...",
  // This is not used -- in fact, { namingStrategy: 5 } has the same results!
  namingStrategy: new MyNamingStrategy(),
  ...
});

[pleerock]

Confirmed. I'll create a PR for this issue.

[pleerock]

fixed by #8764