fix: change OrmUtils.mergeDeep to not merge RegExp objects #5182
Conversation
tobyhinloopen
changed the title
Before entity column values are transformed, changes are deeply merged using OrmUtils.mergeDeep. This mergeDeep function attempts to merge objects, but wrongly attempted to merge RegExp objects. This merging of RegExp objects breaks the object, rendering them unusable. This commit changes mergeDeep to not merge RegExp objects but overwrite them instead. Closes typeorm#3534
tobyhinloopen
force-pushed
the
dont-clone-regexp
branch
from
7dcd3c9
to
bbd38ff
Compare
| before(async () => { | ||
| connections = await createTestingConnections({ | ||
| entities: [__dirname + '/entity/*{.js,.ts}'], | ||
| enabledDrivers: ['postgres'], |
There was a problem hiding this comment.
can you please enable all drivers? since we don't have anything postgres specific
| connections = await createTestingConnections({ | ||
| entities: [__dirname + '/entity/*{.js,.ts}'], | ||
| enabledDrivers: ['postgres'], | ||
| dropSchema: true, |
There was a problem hiding this comment.
why do we need it? (please remove if we don't)
| after(() => closeTestingConnections(connections)); | ||
|
|
||
| it('allows entities with regexp columns', () => PromiseUtils.runInSequence(connections, async connection => { | ||
| await connection.synchronize(); |
There was a problem hiding this comment.
we don't usually do it this way... can you please remove it? (just take a look on other tests how we do it in most of the times)...
There was a problem hiding this comment.
Im just trying to find why tests are passed but CI failed. Usually it happens on a "stucked promise", but I can't easily find the problem in this test
|
Looks like a valid feature request, we just need to find out why CI failed. |
|
I applied few changes let's see if tests won't fail. |
|
@pleerock Thanks for your review. I'll fix any remaining issues / feedback one of the next (work)days. :) |
|
@pleerock - The CI is 🍏 |
|
Thanks! |
Closes #3534
Before entity column values are transformed, changes are deeply merged using OrmUtils.mergeDeep. This mergeDeep function attempts to merge objects, but wrongly attempted to merge RegExp objects. This merging of RegExp objects breaks the object, rendering them unusable. This commit changes mergeDeep to not merge RegExp objects but overwrite them instead.
To cleanup the mergeDeep function, I also refactored
propertyKeyaway, and I changed all references tosource[key]orsource[propertyKey]tovalue, also removing the commented code.