Skip to content

Change service_data to just data#12628

Merged
zsarnett merged 6 commits intodevfrom
service-data-to-data
May 24, 2022
Merged

Change service_data to just data#12628
zsarnett merged 6 commits intodevfrom
service-data-to-data

Conversation

@thomasloven
Copy link
Contributor

@thomasloven thomasloven commented May 8, 2022

Proposed change

In the spirit of streamlining and designing away the most common user errors.

Does anyone even remember why it's service_data in the frontend, if there ever was a reason?

Edit: For future reference; It's probably because tap_action used to just be a string "call-service" and then service and service_data were parameters on the element itself rather than everything being neatly tucked into a tap_action object as it is now. In that case data would be a terribly broad keyword to reserve.

Should be backwards compatible.

Todo:

  • Architecture discussion??
  • Actually run this and see if it works
  • Documentation
  • Update gallery
  • Automatically convert/merge service_data into data in GUI editors.

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

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:

@thomasloven thomasloven changed the title Change service_data to just data RFC: Change service_data to just data May 9, 2022
@bramkragten
Copy link
Member

Yes, yes, yes!

@thomasloven
Copy link
Contributor Author

I'll just go ahead then.

Copy link
Member

@spacegaier spacegaier left a comment

Choose a reason for hiding this comment

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

To prevent confusion 😄. There are a few more places where such a comment might make sense.

service_data?: {
[key: string]: any;
};
service_data?: Record<string, unknown>;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
service_data?: Record<string, unknown>;
// "service_data" is kept to be backwards-compatible. New coding should be using "data" instead.
service_data?: Record<string, unknown>;

@@ -55,6 +55,7 @@ export interface EntitiesCardEntityConfig extends EntityConfig {
action_name?: string;
service?: string;
service_data?: Record<string, unknown>;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
service_data?: Record<string, unknown>;
// "service_data" is kept to be backwards-compatible. New coding should be using "data" instead.
service_data?: Record<string, unknown>;

@spacegaier
Copy link
Member

Is that something where similarly to the way we handle core integrations we want to deprecate that in half a year or so and force users to switch to data so we do not have to carry the coding "mixture" forever?

@frenck
Copy link
Member

frenck commented May 9, 2022

Is that something where similarly to the way we handle core integrations we want to deprecate that in half a year or so and

In core we had data and data_template; we still carry them around and have been removed over 2 years ago. Up until this day, I still see new examples, issues, forum, and Discord messages using the old data_template.

I guess this is similar; which probably should stick for a long long time.

@thomasloven thomasloven force-pushed the service-data-to-data branch from 50e3f4f to b5e9e79 Compare May 21, 2022 18:03
@thomasloven thomasloven added the needs design preview PRs with this label will trigger a GitHub action to generate a gallery preview label May 21, 2022
@thomasloven thomasloven marked this pull request as ready for review May 21, 2022 18:05
@thomasloven thomasloven changed the title RFC: Change service_data to just data Change service_data to just data May 21, 2022
thomasloven added a commit to home-assistant/home-assistant.io that referenced this pull request May 21, 2022
@thomasloven
Copy link
Contributor Author

thomasloven commented May 22, 2022

I'm thinking maybe service_data should be kept for the service call button element since that doesn't have a tap_action.
At least until it is deprecated in favor of a more generic button element.

The thought being that maybe we want a data attribute for all elements at some point in the future.

Thoughts?

@zsarnett
Copy link
Contributor

Agreed. Can leave for now and then deprecate

@zsarnett zsarnett merged commit e3d394e into dev May 24, 2022
@zsarnett zsarnett deleted the service-data-to-data branch May 24, 2022 15:49
frenck pushed a commit to home-assistant/home-assistant.io that referenced this pull request May 24, 2022
@github-actions github-actions bot locked and limited conversation to collaborators May 25, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

cla-signed needs design preview PRs with this label will trigger a GitHub action to generate a gallery preview

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants