1Password / op-js

A JS library powered by the 1Password CLI
https://developer.1password.com/docs/cli
MIT License
91 stars 8 forks source link

Section interface should have a `label` field #105

Closed stabai closed 1 year ago

stabai commented 1 year ago

Summary

When working with a Field, the section that field is in has both a label and an id. Even though this data is received from the CLI, the interface doesn't indicate that the label field exists, making it hard to use in TypeScript code.

https://github.com/1Password/op-js/blob/9f6668ba7b125699f6c43aa8a94daa9bcc226b95/src/index.ts#L763

Use cases

If a user wants to look for a specific field within a specific section, they may likely want to use the section label rather than it's ID:

if (field.section?.label === 'EMAIL' && field.label === 'API_KEY') {
  return field.value;
}

Proposed solution

Just add the field to the interface:

export interface Section {
    id: string;
    label: string;
}

It's possible it should be label? rather than label, I'm not sure if the label is always present on a section.

Is there a workaround to accomplish this today?

Currently, users may need to add an awkward type cast to make TypeScript be okay with this:

const fieldSection = field.section as { label?: string } | undefined;
if (fieldSection?.label === 'EMAIL' && field.label === 'API_KEY') {
  return field.value;
}

References & Prior Work

N/A

jodyheavener commented 1 year ago

Hi @stabai! Thanks for filing this issue.

It's possible it should be label? rather than label, I'm not sure if the label is always present on a section.

I think you are correct here and that would be the most appropriate change until we are able to do CLI version-specific releases of op-js. We welcome PRs if you'd like, otherwise I'll add it to the team's radar for a future release!

anujchoudhary-17 commented 1 year ago

Hi @jodyheavener and @stabai, I have raised a PR for this issue. Here is the link: https://github.com/1Password/op-js/pull/133 Please review it and let me know if any changes are required :) Thank you :)

anujchoudhary-17 commented 1 year ago

Hi @jodyheavener and @stabai Raised a new signed PR https://github.com/1Password/op-js/pull/141 please check it :) Thank you