docs(adr): add development-warnings adr

[joshblack]

ADR for Development Warnings in @primer/react

Rendered markdown

Pull Request

[changeset-bot]

⚠️ No Changeset found

Latest commit: 7e42365

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[github-actions]

size-limit report 📦

Path	Size
dist/browser.esm.js	94.49 KB (0%)
dist/browser.umd.js	95.07 KB (0%)

[mperrotti]

I like this direction! Once this merges, could we create an issue to implement the warning() helper?

[colebemis]

I would have guessed that the message would display if the condition is truthy. I read that code as "warn if condition"

If this is a convention, I'm fine to keep it. But I wanted to mention my initial confusion.

If the name of the helper was something like assert(), I think falsey would make more sense.

[broccolinisoup]

I would have guessed that the message would display if the condition is truthy. I read that code as "warn if condition"

Ohh, this is how I think it was as well the entire time - now realised that 🙈

[joshblack]

Totally makes sense! Happy to flip it if that's more intuitive for folks 👍

I think the convention stems from invariant() ultimately, and things like assert(), that frame things from the perspective of "this condition must be met". When it's not met, throw an error, log a warning, etc

To contextualize the two, here is the warning from Heading in the two styles:

// Warn if condition is true
warning(
  innerRef.current && !(innerRef.current instanceof HTMLHeadingElement),
  'This Heading component should be an instanceof of h1-h6'
);

// Warn if condition is false
warning(
  innerRef.current && innerRef.current instanceof HTMLHeadingElement,
  'This Heading component should be an instanceof of h1-h6'
);

[broccolinisoup]

Thanks Josh! Warning when the condition truthy feels more intuitive to me. That said, it is totally cool to me if we want follow the convention that it stems from.

[joshblack]

Let's do truth-y then! I'll update the ADR accordingly 👍

[lesliecdubs]

@joshblack so far it seems most folks are in alignment here, but if you'd like me to assign a decision-maker on this one give me a shout.