Thanks for contribution! Can we have a test that covers these changes?

The thing is, I already tried to make some test cases. But I am not sure how to do a test case for a data model which changes. The only thing I could imagen was to spawn to different node process after each other with different typescript files which represent the model changes. However, I though, man this is dirty.

@TheNoim is this fix compatible with the latest RC builds? I'm running into this exact issue.

I only tested it on 0.2.25, but I think it should. Currently using the patch in production.

@pleerock Any idea how to implement a test for this case? The only idea I came up is: forking the current process to use different typescript classes and enums. However, I think this isn't ideal.

@pleerock Finally, this should be ready for merge.

Any news?

Any news @imnotjames ? Don't want to annoy you, but it feels like forever since I opened this pr.

Can we get some eyes on this? Super annoying bug.

EDIT - confirmed this PR fixes the migrations of array based enums

Any news?

@imnotjames sorry for the ping but could you please re-review?

Maybe @pleerock could look into this?

really need this :-/

Soon I am at the point where I just cry. Both of my prs are stuck for nearly a year.

@TheNoim I'm sorry so much. PR is complex and its hard for me to review it. I had to ask @AlexMesser to help me.

Our conclusion on this PR is: even if it properly works, we aren't sure we want to merge it because of the value/complexity criteria. Let me explain. Right now it's super hard to review and merge changes in schema syncs. It's also hard to make changes to it even if you know (or think you know) how it works. Cost of mistake is high, because breaking change can lead to data loss on user side who didn't backup data before TypeORM upgrade. That's why in this area (rdbms sync) we prefer simple and obvious solutions as much as possible with feature sacrification if needed.

I don't want to say we won't fix this issue, we definitely need to fix it. But we are looking for the most simple approach. To understand if we can make it simpler let me ask first question:

• in TypeORM if we change the column type, e.g. from varchar to int we drop previous column and create a new column. This is implemented this way mostly due to simplicity
• I think if we change from enum to enum array we should consider it same way - its a column type change, and we should drop previous column and create a new column

Can we do it? I think it will lead to more consistent and simpler code with a small feature sacrification. What do you think about it?

You're right to be cautious of the new functionality @pleerock. If it's any consolation, I've used this patch in production and it had the intended effect of migrating the existing enum values to a single member array.

@stewartmcgown thank you, any feedback on PR changes before they being merged are super valuable!

I will comment on this later. I am a bit busy today.

@TheNoim I'm sorry so much. PR is complex and its hard for me to review it. I had to ask @AlexMesser to help me.

Our conclusion on this PR is: even if it properly works, we aren't sure we want to merge it because of the value/complexity criteria. Let me explain. Right now it's super hard to review and merge changes in schema syncs. It's also hard to make changes to it even if you know (or think you know) how it works. Cost of mistake is high, because breaking change can lead to data loss on user side who didn't backup data before TypeORM upgrade. That's why in this area (rdbms sync) we prefer simple and obvious solutions as much as possible with feature sacrification if needed.

I don't want to say we won't fix this issue, we definitely need to fix it. But we are looking for the most simple approach. To understand if we can make it simpler let me ask first question:

• in TypeORM if we change the column type, e.g. from varchar to int we drop previous column and create a new column. This is implemented this way mostly due to simplicity
• I think if we change from enum to enum array we should consider it same way - its a column type change, and we should drop previous column and create a new column

Can we do it? I think it will lead to more consistent and simpler code with a small feature sacrification. What do you think about it?

Isn't this only about the changes I did later on this pr? I mean my original fix only included the pure bug fix for the change of an enum. It didn't include any other fixes like migration from enum to array enum and back. I only started to include this kind of change, because I thought @imnotjames required these. However, after looking through his replies I may misunderstood him. I could revert this changes and make this an pr with only the core bug fix. I mean this is the primary part of the pr: https://github.com/typeorm/typeorm/pull/6126/files#diff-b924b9a6d4ab4328f4ad59d4d96b794f6a1975fae8d1d3708909e31c151730b9R2101

Any thoughts?