Skip to content

fix(docs): Logical sequence for internal components #1845

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

Sonkaryasshu
Copy link

changed internal components sequence to be more logical and easy to follow.

changed internal components sequence to be more logical and easy to follow.
Copy link

changeset-bot bot commented Dec 27, 2024

🦋 Changeset detected

Latest commit: 2752aac

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
react-email Patch

Not sure what this means? Click here to learn what changesets are.

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

Copy link

vercel bot commented Dec 27, 2024

@Sonkaryasshu is attempting to deploy a commit to the resend Team on Vercel.

A member of the Team first needs to authorize it.

@gabrielmfern
Copy link
Collaborator

Why is this easier to follow exactly?

@Sonkaryasshu
Copy link
Author

The updated sequence groups related components logically, making it easier to locate and understand their context. For instance:

  • Structural components like row, column, and section are grouped together in a meaningful order, reflecting the hierarchy typically used when building layouts or following docs(when i realised to correct it).
  • Content-related components like image, text, link, and button are positioned near each other, aligning with how they are often used in tandem.
  • Utility components like code-block, code-inline, and font are placed together to streamline their reference.

This organization reduces cognitive load by reflecting the natural flow of creating email layouts, making the docs more intuitive and efficient to use.

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: gabriel miranda <[email protected]>
@gabrielmfern gabrielmfern force-pushed the canary branch 7 times, most recently from 81c08e3 to 301f54b Compare January 27, 2025 20:07
@gabrielmfern gabrielmfern force-pushed the canary branch 2 times, most recently from a30b3fd to 5f1828b Compare February 4, 2025 15:38
@gabrielmfern gabrielmfern changed the base branch from canary to main February 19, 2025 15:16
Comment on lines 121 to 125
process.env = {
...process.env,
NODE_ENV: 'development',
...(process.env as Omit<NodeJS.ProcessEnv, 'NODE_ENV'> & {
NODE_ENV?: NodeJS.ProcessEnv['NODE_ENV'];
}),
Copy link
Contributor

Choose a reason for hiding this comment

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

The current implementation doesn't respect the user's NODE_ENV as the changeset suggests. It explicitly sets NODE_ENV: 'development' first, then spreads the rest of the environment variables, which means any user-defined NODE_ENV will be overwritten.

To properly respect the user's environment, consider this approach instead:

process.env = {
  ...process.env,
  NODE_ENV: process.env.NODE_ENV || 'development',
}

This preserves the user's NODE_ENV if set, while defaulting to 'development' only when not specified.

Suggested change
process.env = {
...process.env,
NODE_ENV: 'development',
...(process.env as Omit<NodeJS.ProcessEnv, 'NODE_ENV'> & {
NODE_ENV?: NodeJS.ProcessEnv['NODE_ENV'];
}),
process.env = {
...process.env,
NODE_ENV: process.env.NODE_ENV || 'development',
}

Spotted by Diamond

Is this helpful? React 👍 or 👎 to let us know.

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.

None yet

3 participants