Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Favorite column modifies data source but should not interfere #540

Closed
tscherfler opened this issue Feb 4, 2020 · 4 comments · Fixed by #582
Closed

Favorite column modifies data source but should not interfere #540

tscherfler opened this issue Feb 4, 2020 · 4 comments · Fixed by #582
Assignees
Labels
bug A broken behaviour that was working previously has-pr P2 Issue that is important to resolve as soon as possible

Comments

@tscherfler
Copy link
Contributor

tscherfler commented Feb 4, 2020

Bug Report

Expected Behavior

Favorite column should not modify my data source.

Current Behavior

Modifies data by reference.

data[this.name] = !data[this.name];

Possible Solution

Hold favorite state in component itself.

Steps to Reproduce

When data in datasource is immutable this breaks.

Context (Environment)

Used Versions:

  • @dynatrace/barista-components: 5.0.1
@tscherfler tscherfler added the bug A broken behaviour that was working previously label Feb 4, 2020
@tomheller tomheller added the P2 Issue that is important to resolve as soon as possible label Feb 4, 2020
@tomheller
Copy link
Collaborator

tomheller commented Feb 4, 2020

Additionally the example binding is wrongfully called change here:

when the actual output is called favoriteToggled:

@Output() readonly favoriteToggled = new EventEmitter<T>();

@tomheller
Copy link
Collaborator

After a short investigation, this is what I have found:
If the data in the datasource is immutable (the object has been frozen somewhere in the process) we cannot mutate the data on the original object. We as the component providers should not modify the data within that source.

Here is what I would propose as a solution:
In the favorite-colum.ts file

data[this.name] = !data[this.name];

I would remove the modification of the original data.

The modification of the original data is best handled by the consumer of the component library, as we cannot know if the data is mutable or not. They can react to the event and handle the modifications of their data accordingly.

@yngrdyn I would really appreciate your input on this one.

@yngrdyn
Copy link
Contributor

yngrdyn commented Feb 7, 2020

Hello @tomheller, I'll take a look in this

@tomheller
Copy link
Collaborator

Thank you @yngrdyn, you are brilliant 🎉 🤘 .

tomheller pushed a commit to yngrdyn/barista that referenced this issue Feb 13, 2020
yngrdyn added a commit to yngrdyn/barista that referenced this issue Nov 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug A broken behaviour that was working previously has-pr P2 Issue that is important to resolve as soon as possible
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants