Skip to content

Freeze lovelace configuration on load#4862

Merged
balloob merged 5 commits into
devfrom
deep-freeze-config
Feb 14, 2020
Merged

Freeze lovelace configuration on load#4862
balloob merged 5 commits into
devfrom
deep-freeze-config

Conversation

@thomasloven
Copy link
Copy Markdown
Contributor

@thomasloven thomasloven commented Feb 13, 2020

Breaking change

#4851 but with freezing the entire configuration instead of cloning it for each card.

This will break some custom cards, but there's several simple fixes, e.g.:

setConfig(config) {
  config = { ...config };
...
import { deepClone } from "deep-clone-simple";
// https://github.com/balloob/deep-clone-simple

setConfig(config) {
  config = deepClone(config);
...

or

setConfig(config) {
  config = JSON.parse(JSON.stringify(config));
  ...

depending on need and personal preference (besides rethinking the methodology of the card).

Type of change

  • Dependency upgrade
  • Bugfix (non-breaking change which fixes an issue)
  • New feature (thank you!)
  • Breaking change (fix/feature causing existing functionality to break)
  • Code quality improvements to existing code or addition of tests

Example configuration

Additional information

  • This PR fixes or closes issue: fixes #
  • This PR is related to issue:
  • Link to documentation pull request:

Checklist

  • The code change is tested and works locally.
  • There is no commented out code in this PR.
  • Tests have been added to verify that the new code works.

If user exposed functionality or configuration variables are added/changed:

@bramkragten
Copy link
Copy Markdown
Member

I would suggest this as way to copy:

setConfig(config) {
  config = {...config}; // create a mutable copy of the configuration
}

@thomasloven
Copy link
Copy Markdown
Contributor Author

thomasloven commented Feb 13, 2020

That's a shallow copy, though. Isn't it? And ES...5? 6? (non IE)

@bramkragten
Copy link
Copy Markdown
Member

Yes that is shallow, for deep just use the deep-clone from balloob we used before:
https://github.com/balloob/deep-clone-simple/blob/master/index.js

Comment thread src/panels/lovelace/ha-panel-lovelace.ts Outdated
Comment thread src/panels/lovelace/ha-panel-lovelace.ts
Copy link
Copy Markdown
Member

@balloob balloob left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. ok to merge when last comment addressed

@balloob balloob mentioned this pull request Feb 13, 2020
9 tasks
Comment thread src/panels/lovelace/ha-panel-lovelace.ts
Comment thread src/panels/lovelace/ha-panel-lovelace.ts Outdated
@thomasloven
Copy link
Copy Markdown
Contributor Author

A bit of an indication of how ready people are for this can be found in thomasloven/hass-config#6

@balloob balloob merged commit c62a5a6 into dev Feb 14, 2020
@delete-merged-branch delete-merged-branch Bot deleted the deep-freeze-config branch February 14, 2020 00:18
@balloob
Copy link
Copy Markdown
Member

balloob commented Feb 14, 2020

Thanks for going through HACS Thomas. It's not going to be released until 2 weeks from now, hopefully enough time.

checkedConfig.views[index].badges = view.badges.filter(Boolean);
}
});
return checkedConfig ? deepFreeze(checkedConfig) : config;
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This will now only freeze the config when there are faulty badges.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're right...

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lock lock Bot locked and limited conversation to collaborators Feb 15, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants