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

Rename KeyType and JsonWebKey to avoid conflicts with predefined types #17710

Closed
maorleger opened this issue Sep 15, 2021 · 4 comments
Closed
Assignees
Labels
Client This issue points to a problem in the data-plane of the library. KeyVault

Comments

@maorleger
Copy link
Member

When upgrading API Extractor we noticed that KeyType and JsonWebKey defined in KV Keys conflict with the same names in dom.lib.ts which causes them to be renamed as KeyType_2 and JsonWebKey_2 in the API view.

To avoid this conflict, we can rename them locally but still export the conflicted name as deprecated alias.

Open questions:

  1. What should we rename these to
  2. What should we do with other names such as KnownKeyTypes? Rename them as well?

Created as a result of https://github.com/Azure/azure-sdk-for-js/pull/17702/files#r709589284

@maorleger maorleger added Client This issue points to a problem in the data-plane of the library. KeyVault labels Sep 15, 2021
@maorleger maorleger added this to the Backlog milestone Sep 15, 2021
@maorleger maorleger self-assigned this Sep 15, 2021
@xirzec
Copy link
Member

xirzec commented Sep 15, 2021

I feel like this is, similar to a new lint check, finding a problem that we have today. I would prefer to never shadow a global type in the runtime environments we support if we can avoid it.

That said, it's not hard for consumers to alias our poor names to something unambiguous.

@ramya-rao-a
Copy link
Contributor

Before taking this up, I would be curious to do a sweep across all our packages and collect other such references to get an idea of the extent of this problem

@maorleger
Copy link
Member Author

Just a note that this and #17721 are very closely related - in JS's case there's no serialization really and the only difference between our JsonWebKey and the built-in is that we use keyOps instead of key_ops. Other languages have more differences but for us it might be as simple as deprecating keyOps (but not removing it until the next major) and starting to use key_ops.

Copy link

Hi @maorleger, we deeply appreciate your input into this project. Regrettably, this issue has remained inactive for over 2 years, leading us to the decision to close it. We've implemented this policy to maintain the relevance of our issue queue and facilitate easier navigation for new contributors. If you still believe this topic requires attention, please feel free to create a new issue, referencing this one. Thank you for your understanding and ongoing support.

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Mar 15, 2024
@github-actions github-actions bot locked and limited conversation to collaborators Mar 15, 2024
@xirzec xirzec removed this from the Backlog milestone May 7, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Client This issue points to a problem in the data-plane of the library. KeyVault
Projects
Archived in project
Development

No branches or pull requests

4 participants