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

rfc: Deprecate Custom Element Attribute Enums? #5765

Closed
nicholasrice opened this issue Mar 21, 2022 · 5 comments · Fixed by #5930
Closed

rfc: Deprecate Custom Element Attribute Enums? #5765

nicholasrice opened this issue Mar 21, 2022 · 5 comments · Fixed by #5930
Assignees
Labels
breaking-change A breaking change to a shipping package community:noteworthy An issue or PR of particular interest to the community or planned for an announcement. community:request Issues specifically reported by a member of the community. improvement A non-feature-adding improvement

Comments

@nicholasrice
Copy link
Contributor

#5745 introduces string literal type unions for custom element HTML attributes where they exist in the FAST component packages.

This issue tracks an open question from that PR is if the enum exports that exist for these attributes should be deprecated.

@nicholasrice nicholasrice added the status:triage New Issue - needs triage label Mar 21, 2022
@chrisdholt
Copy link
Member

Would be a welcome change IMO. In most cases I would prefer an alternative to enums, personally. One thing I was considering was if a const could avoid the removal of the enums without a break and pave the way for this change over time…even that may not be necessary though…

@EisenbergEffect EisenbergEffect added improvement A non-feature-adding improvement area:fast-components community:request Issues specifically reported by a member of the community. community:noteworthy An issue or PR of particular interest to the community or planned for an announcement. and removed status:triage New Issue - needs triage labels Mar 23, 2022
@EisenbergEffect EisenbergEffect added the breaking-change A breaking change to a shipping package label Mar 23, 2022
@radium-v
Copy link
Collaborator

Enums can be replaced with objects: https://www.typescriptlang.org/docs/handbook/enums.html#objects-vs-enums

@radium-v
Copy link
Collaborator

It can be super useful to have explicit types available as values, especially in tests and Storybook fixtures. You can't use Object.values(SomeList) with union types.

@rajsite
Copy link

rajsite commented Apr 25, 2022

I think switching enums to objects would work well together with template literal types to create enumerable objects and create a union type that is easy to maintain:

const PositionOptions = {
  Up: 'up',
  Down:'down',
  Left: 'left',
  Right:'right',
} as const;

type Position = `${typeof PositionOptions[keyof typeof PositionOptions]}`;

playground

@chrisdholt chrisdholt self-assigned this May 3, 2022
@chrisdholt
Copy link
Member

I have a PR for this and as illustrated by @rajsite I think this is going to be essentially transparent. The types will exist so types shouldn't break and importing the const should be able to be used like the enum was.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change A breaking change to a shipping package community:noteworthy An issue or PR of particular interest to the community or planned for an announcement. community:request Issues specifically reported by a member of the community. improvement A non-feature-adding improvement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants