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(schema): add ability to ignore specific column changes #3503

Merged
merged 2 commits into from
Sep 14, 2022

Conversation

PenguinToast
Copy link
Contributor

@PenguinToast PenguinToast commented Sep 14, 2022

@Property({
  columnType: 'timestamp',
  extra: 'VIRTUAL GENERATED',
  ignoreSchemaChanges: ['type', 'extra'],
})
changingField!: Date;

This is useful for situations such as #1904, where knex is unable to properly diff the column.

Fixes #1904

```ts
@Property({
  columnType: 'timestamp',
  extra: 'VIRTUAL GENERATED',
  ignoreSchemaChanges: ['type', 'extra'],
})
changingField!: Date;
```

This is useful for situations such as mikro-orm#1904, where `knex` is unable to
properly diff the column.
Copy link
Member

@B4nan B4nan left a comment

Choose a reason for hiding this comment

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

Looking great, thanks!

Two notes:

  • are we sure ignoring type and extra props is enough? we can always add more, so not a big deal, and I guess the type here is the most important part
  • let's document this somewhere, I would go with the https://mikro-orm.io/docs/schema-generator and https://mikro-orm.io/docs/defining-entities pages, cant be very brief note about this, but it should be there, with a small example (with the generated column, that's nice and probably the most important use case for this)

@codecov-commenter
Copy link

codecov-commenter commented Sep 14, 2022

Codecov Report

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

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

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #3503   +/-   ##
=======================================
  Coverage   99.85%   99.85%           
=======================================
  Files         210      210           
  Lines       12906    12914    +8     
  Branches     2984     2992    +8     
=======================================
+ Hits        12887    12895    +8     
  Misses         19       19           
Impacted Files Coverage Δ
packages/core/src/decorators/Property.ts 100.00% <ø> (ø)
packages/core/src/typings.ts 100.00% <ø> (ø)
packages/knex/src/schema/DatabaseTable.ts 100.00% <ø> (ø)
packages/knex/src/schema/SchemaComparator.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.

@PenguinToast
Copy link
Contributor Author

@B4nan wow, thanks for the quick review!

Added a commit w/ the documentation changes. As for your other comment, when I tested this change against our database, ignoring type and extra changes seemed to be enough (mikro-orm schema:update --dump no longer generated any statements).

@B4nan B4nan merged commit 05fb1ce into mikro-orm:master Sep 14, 2022
@B4nan
Copy link
Member

B4nan commented Sep 14, 2022

@PenguinToast
Copy link
Contributor Author

FYI there are some broken links

https://github.com/mikro-orm/mikro-orm/actions/runs/3051149375/jobs/4919032117

Ah, my bad -- thanks for the fix!

@PenguinToast PenguinToast deleted the willsheu/gh1904 branch September 14, 2022 16:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Workaround for virtual columns
3 participants