From 321801aca743c519bd2dd40fe0f49503f2f60a26 Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 22 Feb 2023 11:49:28 -0600 Subject: [PATCH 1/4] docs(adr): add development-warnings adr --- .../adrs/adr-XXX-development-warnings.md | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 contributor-docs/adrs/adr-XXX-development-warnings.md diff --git a/contributor-docs/adrs/adr-XXX-development-warnings.md b/contributor-docs/adrs/adr-XXX-development-warnings.md new file mode 100644 index 00000000000..6bd2bfc88e6 --- /dev/null +++ b/contributor-docs/adrs/adr-XXX-development-warnings.md @@ -0,0 +1,59 @@ +# ADR XXX: Development Warnings + +## Status + +Proposed + +## Context + +There are situations where we would like to provide warnings to developers who +use `@primer/react` that something may be deprecated, unsupported, etc. Often, +these will be emitted using `console.warn()`. When using `console.warn()` by +itself, we run into a situation where code that is only be meant for development +is included in production code. As a result, it would be helpful to establish +patterns around how to provide warnings to developers in order to make sure +that: + +- Calls to `console.warn()` do not appear in production +- Code related to development checks, warnings, or messages are removed from + production code + +## Decision + +Code that is meant for development-only warnings **must** be wrapped within a +`__DEV__` block. + +```tsx +function ExampleComponent() { + if (__DEV__) { + // This code only runs in development + } +} +``` + +Under-the-hood, the `__DEV__` block will be compiled to a `NODE_ENV` check so +that it is stripped when `NODE_ENV` is `production`. + +> **Note** +> Contributors may wrap hooks within a `__DEV__` block even though hooks are not +> meant to be called conditionally. This is because the `__DEV__` check can be +> considered constant in that it will always be true or false for the +> environment (development or production). + +When a contributor would like to communicate a warning to a developer, they +should use the `warning()` helper. + +```ts +warning(condition, 'This is the message that is logged when condition is false-y') +``` + +This helper allows you to provide a `condition`. When the condition is false-y +it will emit the message provided. + +For more complex conditions, a contributor may combine `console.warn()` with +`__DEV__` when `warning()` does not suffice. + +### Impact + +- Calls to `console.warn()` will be replaced by `warning()` or will be wrapped + in a `__DEV__` block From 9bec22eaed33871c0064e2ae5593e460ffbdf15a Mon Sep 17 00:00:00 2001 From: Josh Black Date: Wed, 22 Feb 2023 12:04:49 -0600 Subject: [PATCH 2/4] Update adr-XXX-development-warnings.md --- contributor-docs/adrs/adr-XXX-development-warnings.md | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/contributor-docs/adrs/adr-XXX-development-warnings.md b/contributor-docs/adrs/adr-XXX-development-warnings.md index 6bd2bfc88e6..9c2dbfdf930 100644 --- a/contributor-docs/adrs/adr-XXX-development-warnings.md +++ b/contributor-docs/adrs/adr-XXX-development-warnings.md @@ -9,7 +9,7 @@ Proposed There are situations where we would like to provide warnings to developers who use `@primer/react` that something may be deprecated, unsupported, etc. Often, these will be emitted using `console.warn()`. When using `console.warn()` by -itself, we run into a situation where code that is only be meant for development +itself, we run into a situation where code that is only meant for development is included in production code. As a result, it would be helpful to establish patterns around how to provide warnings to developers in order to make sure that: @@ -20,7 +20,7 @@ that: ## Decision -Code that is meant for development-only warnings **must** be wrapped within a +Code that is meant for development-only warnings or checks **must** be wrapped within a `__DEV__` block. ```tsx @@ -32,7 +32,7 @@ function ExampleComponent() { ``` Under-the-hood, the `__DEV__` block will be compiled to a `NODE_ENV` check so -that it is stripped when `NODE_ENV` is `production`. +that it is stripped when `NODE_ENV` is set to `'production'`. > **Note** > Contributors may wrap hooks within a `__DEV__` block even though hooks are not @@ -48,7 +48,8 @@ warning(condition, 'This is the message that is logged when condition is false-y ``` This helper allows you to provide a `condition`. When the condition is false-y -it will emit the message provided. +it will emit the message provided. This helper is automatically wrapped in a +`__DEV__` block and will be removed from production builds. For more complex conditions, a contributor may combine `console.warn()` with `__DEV__` when `warning()` does not suffice. From f6b47bc64bf43f0d08c2ca505894662445e78a2d Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 28 Feb 2023 10:16:38 -0600 Subject: [PATCH 3/4] chore: update false-y to truth-y for warning check --- ...elopment-warnings.md => adr-012-development-warnings.md} | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) rename contributor-docs/adrs/{adr-XXX-development-warnings.md => adr-012-development-warnings.md} (97%) diff --git a/contributor-docs/adrs/adr-XXX-development-warnings.md b/contributor-docs/adrs/adr-012-development-warnings.md similarity index 97% rename from contributor-docs/adrs/adr-XXX-development-warnings.md rename to contributor-docs/adrs/adr-012-development-warnings.md index 9c2dbfdf930..b40f85835c0 100644 --- a/contributor-docs/adrs/adr-XXX-development-warnings.md +++ b/contributor-docs/adrs/adr-012-development-warnings.md @@ -2,7 +2,7 @@ ## Status -Proposed +Accepted ## Context @@ -44,10 +44,10 @@ When a contributor would like to communicate a warning to a developer, they should use the `warning()` helper. ```ts -warning(condition, 'This is the message that is logged when condition is false-y') +warning(condition, 'This is the message that is logged when condition is truth-y') ``` -This helper allows you to provide a `condition`. When the condition is false-y +This helper allows you to provide a `condition`. When the condition is truth-y it will emit the message provided. This helper is automatically wrapped in a `__DEV__` block and will be removed from production builds. From 7e423651a13836a1ecebd0198c2d939fa37f027b Mon Sep 17 00:00:00 2001 From: Josh Black Date: Tue, 28 Feb 2023 10:17:31 -0600 Subject: [PATCH 4/4] chore: update title --- contributor-docs/adrs/adr-012-development-warnings.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/contributor-docs/adrs/adr-012-development-warnings.md b/contributor-docs/adrs/adr-012-development-warnings.md index b40f85835c0..b147d19cd8f 100644 --- a/contributor-docs/adrs/adr-012-development-warnings.md +++ b/contributor-docs/adrs/adr-012-development-warnings.md @@ -1,4 +1,4 @@ -# ADR XXX: Development Warnings +# ADR 012: Development Warnings ## Status