feat(entity-generator): generate OptionalProps symbols

[rhyek]

…ies that define a default value

[codecov-commenter]

Codecov Report

Base: 99.89% // Head: 99.89% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (0d843d0) compared to base (df7d939).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff           @@
##           master    #3482   +/-   ##
=======================================
  Coverage   99.89%   99.89%           
=======================================
  Files         205      205           
  Lines       12814    12822    +8     
  Branches     2972     2974    +2     
=======================================
+ Hits        12801    12809    +8     
  Misses         13       13           

Impacted Files	Coverage Δ
packages/entity-generator/src/SourceFile.ts	100.00% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[B4nan]

Just one nit, otherwise looking good, thanks!

Btw this PR got me thinking, maybe its about time to provide @mikro-orm/client package or something similar with schema first codegen approach.

[B4nan]

the symbol can/should be defined as optional

Suggested change

	ret += `\n\n${' '.repeat(2)}[OptionalProps]: ${optionalPropertyNames.join(' | ')};`;
	ret += `\n\n${' '.repeat(2)}[OptionalProps]?: ${optionalPropertyNames.join(' | ')};`;

(this will require regenererating snapshots)

[B4nan]

Suggested change

	if (optionalProperties.length > 0) {
	if (optionalProperties.length > 0) {

[B4nan]

Btw I did some PoC of that generated client I mentioned in the tweet, and found few more things we first need to support in the generator. If you like this part of the codebase, I'll be happy to accept PRs for following:

• generate simple repositories next to the entities (in separate files, linking them via EntityRepositoryType symbol)
• allow to generate JS/D.TS, again in separate files - for code gen we would have to go with this approach probably
• in the generated repository, we should extend the driver package, for that we will need a way to get package name and driver class (there is no way currently)

Last step would be that "generated client". I have this simple script I used for the PoC, it generates entities, repositories and a client file which initializes the ORM and exports a map of services:

import { MikroORM } from '@mikro-orm/core';
import { dirname } from 'node:path';
import fs from 'fs-extra';
import { fileURLToPath } from 'node:url';

// TODO
//  - generate repositories in separate files
//  - provide way to generate .js + .d.ts files
//  - generate experimental client
//  - allow detecting m:n with AI PK
//  - import EM/repo from driver package instead of core

const orm = await MikroORM.init();
const dirName = dirname(fileURLToPath(import.meta.url));
const driver = { package: '@mikro-orm/better-sqlite', className: 'BetterSqliteDriver' };

// clean up everything in the folder except dot files
await fs.emptyDir(dirName + '/entities');
await fs.emptyDir(dirName + '/repositories');

const entityGenerator = orm.config.get('entityGenerator');
entityGenerator.identifiedReferences = true;
entityGenerator.bidirectionalRelations = true;
// orm.config.set('entityGenerator', entityGenerator);
const ret = await orm.entityGenerator.generate({
  baseDir: dirName + '/entities',
  save: true,
});

const metadata = orm.getMetadata().getAll();
const entities = Object.values(metadata).filter(meta => !meta.pivotTable && !meta.embeddable && !meta.virtual);

for (const meta of entities) {
  const code = [
    `import { EntityRepository } from '${driver.package}';`,
    `import { ${meta.className} } from '../entities/${meta.className}.js';`,
    '',
    `export class ${meta.className}Repository extends EntityRepository<${meta.className}> { }`,
  ];
  await fs.writeFile(`${dirName}/repositories/${meta.className}Repository.ts`, code.join('\n'));
}

const client: string[] = [];
const coreImports: string[] = [];

client.push(`import { ${driver.className} } from '${driver.package}';`);

for (const meta of entities) {
  client.push(`import { ${meta.className} } from './entities/${meta.className}.js';`);
  client.push(`import { ${meta.className}Repository } from './repositories/${meta.className}Repository.js';`);
}

client.push('');
coreImports.push('MikroORM');
client.push(`const orm = await MikroORM.init<${driver.className}>();`);
// we will need something like this, but that itself won't allow extension, we would have to check if something was
// discovered, discover the generated entities only if they are not provided and discovered already. user would extend
// the generated entities, this means we might have to mark them as base entities somehow, to get around duplicity warnings
client.push(`await orm.discoverEntity([${entities.map(meta => meta.className).join(', ')}]);`);
client.push(`const client = {`);
client.push(`  orm,`);
client.push(`  em: orm.em,`);
client.push(`  schema: orm.schema,`);
client.push(`  seeder: orm.seeder,`);
client.push(`  migrator: orm.migrator,`);

function lcfirst(word: string) {
  return word[0].toLowerCase() + word.substring(1);
}

for (const meta of entities) {
  client.push(`  ${lcfirst(meta.className)}: orm.em.getRepository(${meta.className}) as ${meta.className}Repository,`);
}

client.push(`};`);
client.push(`export default client;`);
client.push('');

client.unshift(`import { ${coreImports.join(', ')} } from '@mikro-orm/core';`);

console.log(client.join('\n'));
await fs.writeFile(dirName + '/client.ts', client.join('\n'));

console.info('Database client generated');
await orm.close();

This would work only when used with the generated entities, important bit here is finding a way how to make this work dynamically with user provided entitites (that would extend the generated ones).

[rhyek]

@B4nan thanks for merging. i am still getting familiar with this package's code. could potentially help with your idea, but would need to understand the use-case first. i saw you mentioned schema-first code generation like prisma. sounds interesting. i will say i've tried prisma briefly and am not completely sold on the idea of schema-first for an orm. in the case of prisma it is a new domain-specific language that they needed to support and a language-server to support it (not sure if first or third-party).

anyways. i might not have understood your idea, correctly. i'll read through your tweet later. kinda swamped with work atm, heh. i'm sure that is familiar territory for you.

[rhyek]

i had another observation. i noticed that for nullable columns you are generating:

@Property({ nullable: true })
aProperty?: string;

should that be aProperty?: string | null, instead? i have strict null checks enabled for my projects and noticed this was an issue for me today.

[B4nan]

should that be aProperty?: string | null, instead? i have strict null checks enabled for my projects and noticed this was an issue for me today.

This really depends on the user, personally I dont like to define properties that way, it breaks reflect-metadata, and to be consistent, it kinda requires the property initializer. I dont mind having a config toggle for this, but I dont think we should change the defaults now. But maybe I just had some kind of aversion to this, in context of entity generator we can easily provide the type explicitly (so we dont have to care about reflect-metadata at all). I guess the form I dislike is aProperty: string | null = null, which some people use I believe, and is not 100% precise either.

Btw I made an issue based on the tweet, with initial PoC, maybe that will help you understand better: #3484

The point is to have the very same ORM support, just without creating the entities manually. Kinda like the prisma client, we would generate the entities (probably via CLI), the question is where and how it would work with IDE support if we put it into node_modules. For users this would work as usually, with one exception, there would be new package they would import this "client" from, and they would just use it, no need to init the ORM, register DI, care about imports, etc. It would be another way to use the ORM, for simpler projects.

import { client } from '@mikro-orm/client';

const user = client.user.create({ name: '...', email: '...' });
await client.em.persist(user).flush();

Things like this would be possible, without much setup, just the ORM config so the CLI knows what to build. The client object would be fully typed, client.user being a User repository instance. For this to make sense, we would need to find a way to let users provide their own entities (and extend the ones generated by the ORM probably). I think it should be doable, the interface might not be great, but it will unlock new ways and that is important bit. Imagine you would just add something like generator: { customEntity: { user: MyUser } } or similar - which would map your own MyUser entity on top of what the ORM generates for user table. You could skip all the boring code, all properties would be automatically there.

[rhyek]

@B4nan, for

`first_name` varchar(255) null

would this make the most sense?

[OptionalProps]?: 'firstName';

@Property({ type: 'varchar', length: 255, nullable: true })
firstName!: string | null;

[B4nan]

If you want to use the null type explicitly, i'd also add a runtime default:

@Property({ type: 'varchar', length: 255, nullable: true })
firstName: string | null = null;

Personally I just go with optional properties and don't use null explicitly, I know its not 100% correct but its much more ergonomic.

[rhyek]

My thinking is I want to represent more accurately what the orm will return once I run a select query. The = null can be useful when creating entities (and before persisting), but to be consistent I'd then want to also do stuff like:

@Property({ type: 'varchar', length: 255, nullable: true, default: 'hello' })
firstName: string | null ='hello';

Might be problematic for many cases.

[B4nan]

Keep in mind that you can still end up with undefined anywhere if you use partial loading. Another option is the opposite direction, enforcing undefined instead of null via forceUndefined: true.

[B4nan]

Oh we are talking about what the entity generator does? I thought its a general question. I dont mind supporthing this in the generor, but rather opt-in. It should already produce the correct database default too.

[rhyek]

What do you mean by "It should already produce the correct database default too."? Can you give an example?

[B4nan]

https://github.com/mikro-orm/mikro-orm/blob/master/tests/features/entity-generator/__snapshots__/EntityGenerator.test.ts.snap#L15-L16

But I guess we should also do the runtime default there for such properties. In latest version (maybe not yet released) its also possible to infer the runtime defaults (as long as the entity instance can be created without any arguments via constructor).

edit: I guess its only about string defaults https://github.com/mikro-orm/mikro-orm/blob/master/tests/features/entity-generator/__snapshots__/EntityGenerator.test.ts.snap#L790-L794