Skip to content

Conversation

@kellyjosephprice
Copy link
Contributor

🚥 Resolves ISSUE_ID

🧰 Changes

Allows the use of the prop: "name".

<Variable name="apiKey" />

Since this is going to be a "public" component soon, we thought a less repetitive prop would be appropraite?

🧬 QA & Testing

  • tests pass?

@kellyjosephprice kellyjosephprice marked this pull request as ready for review May 22, 2024 22:41
index.jsx Outdated
}

getName() {
return this.props.name || this.props.variable;
Copy link
Member

Choose a reason for hiding this comment

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

does this need a third fallback in case name and variable are both missing?

Copy link
Member

Choose a reason for hiding this comment

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

maybe itll be okay because doing arr[undefined] just returns undefined.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It ought to be a runtime or compile time error? Should I throw like a Props missing error?

Copy link
Member

Choose a reason for hiding this comment

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

ya that might be a good idea

kellyjosephprice and others added 2 commits May 22, 2024 15:47
Co-authored-by: Jon Ursenbach <erunion@users.noreply.github.com>
@kellyjosephprice kellyjosephprice merged commit 019756f into main May 22, 2024
@kellyjosephprice kellyjosephprice deleted the different-prop branch May 22, 2024 22:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants