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

Source object is frozen when mapped #467

Closed
2 of 9 tasks
LennartH opened this issue Apr 21, 2022 · 1 comment · Fixed by #468
Closed
2 of 9 tasks

Source object is frozen when mapped #467

LennartH opened this issue Apr 21, 2022 · 1 comment · Fixed by #468
Labels
bug Something isn't working

Comments

@LennartH
Copy link
Contributor

LennartH commented Apr 21, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Describe the issue

The source object is frozen after it has been mapped, resulting in errors like TypeError: Cannot assign to read only property 'name' of object '#<Foo>' when trying to change the source afterwards. Probably related to #464.

Models/DTOs/VMs

No response

Mapping configuration

No response

Steps to reproduce

import { AutoMap, classes } from '@automapper/classes';
import { CamelCaseNamingConvention, createMap, createMapper, Mapper } from '@automapper/core';

class Foo {
  @AutoMap()
  id: string;
  @AutoMap()
  name: string;
}

class Bar {
  @AutoMap()
  id: string;
  @AutoMap()
  name: string;
}

export const mapper: Mapper = createMapper({
  strategyInitializer: classes(),
  namingConventions: new CamelCaseNamingConvention(),
});
createMap(mapper, Foo, Bar)

const foo = new Foo();
foo.id = 'id';
foo.name = 'foo';
const bar = mapper.map(foo, Foo, Bar);
console.log(bar);

console.log(Object.isFrozen(foo))
foo.name = 'baz';
Output
Bar { name: 'foo', id: 'id' }
true
TypeError: Cannot assign to read only property 'name' of object '#<Foo>'
    at file:///home/lennart/projects/ace/ace-alternatives/investor-navigator/shared-model/scripts/playground.ts:31:9
    at ModuleJob.run (node:internal/modules/esm/module_job:197:25)
    at async Promise.all (index 0)
    at async ESMLoader.import (node:internal/modules/esm/loader:337:24)
    at async loadESM (node:internal/process/esm_loader:88:5)
    at async handleMainPromise (node:internal/modules/run_main:61:12)

Expected behavior

The source object should be mutable after it was mapped.

Screenshots

No response

Minimum reproduction code

No response

Package

  • I don't know.
  • @automapper/core
  • @automapper/classes
  • @automapper/nestjs
  • @automapper/pojos
  • @automapper/mikro
  • @automapper/sequelize
  • Other (see below)

Other package and its version

No response

AutoMapper version

8.3.3

Additional context

No response

@LennartH LennartH added the bug Something isn't working label Apr 21, 2022
@LennartH
Copy link
Contributor Author

I think the issue is caused by the mapper proxy:

// packages/core/src/lib/core.ts - Mapper.map
if (sourceObject == null)
    return sourceObject as TDestination;

const mapping = getMapping(
    receiver,
    sourceIdentifier,
    destinationIdentifier
);

sourceObject = strategy.preMap(
    Object.freeze(sourceObject),
    mapping
);

const destination = mapReturn(
    mapping,
    sourceObject,
    options || {}
);

return strategy.postMap(
    Object.freeze(sourceObject),
    // seal destination so that consumers cannot add properties to it
    // or change the property descriptors. but they can still modify it
    // the ideal behavior is seal but the consumers might need to add/modify the object after map finishes
    destination,
    mapping
);

It looks like the source frozen to prevent side effects by pre-/postMap, but according to the documentation of Object.freeze() it doesn't create a frozen copy but freezes the object in place:

freeze() returns the same object that was passed into the function. It does not create a frozen copy.

I think the source should be copied before freezing or freezing the source should be omitted completely leaving it to the user to make sure that the pre and post methods are side effect free.

LennartH added a commit to LennartH/mapper that referenced this issue Apr 21, 2022
Prevent freezing of the given source object as side effect of map(Array)/mutate(Array).
Create shallow copy of source before freezing it and using the frozen copy for the remaining
mapping process.

ISSUES CLOSED: nartc#467
LennartH added a commit to LennartH/mapper that referenced this issue May 6, 2022
@nartc nartc closed this as completed in #468 May 6, 2022
nartc pushed a commit that referenced this issue May 6, 2022
* fix(core): fix freezing of source object after mapping

Prevent freezing of the given source object as side effect of map(Array)/mutate(Array).
Create shallow copy of source before freezing it and using the frozen copy for the remaining
mapping process.

ISSUES CLOSED: #467

* cleanup(core): remove unused imports in test file

* fix(core): remove freezing source object during mapping

ISSUES CLOSED: #467
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant