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

Redundant call to mapping.mapRawDataToObjectProperty if mapping result isn't a Promise #2013

Open
marchant opened this issue Apr 1, 2019 · 1 comment
Assignees

Comments

@marchant
Copy link
Member

marchant commented Apr 1, 2019

data-service.js line 1424 in _fetchObjectPropertyWithPropertyDescriptor:

-> result = mapping.mapRawDataToObjectProperty(data, object, propertyName);

This was already done a few lines up:
1415: result = mapping.mapObjectToCriteriaSourceForProperty(object, data, propertyName);

Why are we re-doing this?

Need to test removing it and make sure there are no regressions.

@tejaede
Copy link
Collaborator

tejaede commented Apr 1, 2019

The full snippet in question is the following:

//Why aren't we passing this.snapshotForObject(object) instead of copying everying in a new empty object?
Object.assign(data, this.snapshotForObject(object));
result = mapping.mapObjectToCriteriaSourceForProperty(object, data, propertyName);
if (this._isAsync(result)) {
    return result.then(function() {
        Object.assign(data, self.snapshotForObject(object));
        return mapping.mapRawDataToObjectProperty(data, object, propertyName);
    });
} else {
    //This was already done a few lines up. Why are we re-doing this?
    Object.assign(data, self.snapshotForObject(object));
    result = mapping.mapRawDataToObjectProperty(data, object, propertyName);
    if (!this._isAsync(result)) {
        result = this.nullPromise;
    }
    return result;
}

I believe the redundancy is the second call to Object.assign(data, self.snapshotForObject(object));. Aside from that, no logical path results in duplicate calls to the same method.

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

No branches or pull requests

2 participants