mikro-orm icon indicating copy to clipboard operation
mikro-orm copied to clipboard

feat(core): added Platform.getDefaultVarcharLength and optional Type.getDefaultLength

Open boenrobot opened this issue 1 year ago • 2 comments

The existing types are converted to rely on this method's presence.

The entity generator now omits the length property when it is equal to the default.

The schema generator will not detect a difference between not having length (when that method is defined on the type) and having the length with the default value. As always, specifying columnType bypasses all of that.

boenrobot avatar Jun 25 '24 15:06 boenrobot

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.77%. Comparing base (6255a33) to head (c8a0f9c). Report is 1 commits behind head on master.

Additional details and impacted files
@@           Coverage Diff            @@
##           master    #5749    +/-   ##
========================================
  Coverage   99.77%   99.77%            
========================================
  Files         261      261            
  Lines       18253    18272    +19     
  Branches     3931     4260   +329     
========================================
+ Hits        18212    18231    +19     
  Misses         41       41            

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

codecov[bot] avatar Jun 25 '24 15:06 codecov[bot]

This is the second big thing that was originally included as part of #5684, but since then, I thought of this, which I think is a better long term approach to the default length problem in the entity generator - to let the type declare its default length, and remove the special length handling for types using length, thus allowing any custom type to also utilize length.

~There's still the special case for decimal/numeric, which is also a case that, similarly to #5739, produces migrations even when there were no changes... And I'm not sure why. All the old tests pass, but I added said decimal edge case to tests/features/schema-generator/changing-column-type.mysql.test.ts, for the sake of coverage of the length inference. The inferred length is correctly undefined/null (and no, normalizing both to undefined doesn't help...), so whatever the cause is, it's not length handling related. (EDIT: And in the latest push, I also tweaked the default value to be 1:1 match, so it's not that either...)~

If you wouldn't mind, I'm going to side step the columnType: 'decimal(10,2)' edge case, while still adding coverage... by using SET('one','two') as my example of a column type length inference where the length is undefined. That one does work as expected, i.e. the SET column remains unmodified throughout the test.

boenrobot avatar Jun 27 '24 08:06 boenrobot