Skip to content
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.

Sign up for GitHub

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

Jump to bottom

feat(templates): bring React 18 templates in line with indie-stack updates #4060

Merged
merged 1 commit into from
Oct 11, 2022
Merged

feat(templates): bring React 18 templates in line with indie-stack updates #4060

merged 1 commit into from
Oct 11, 2022

Conversation

MichaelDeBoey
Copy link
Member

@MichaelDeBoey MichaelDeBoey commented Aug 24, 2022 •
edited
Loading

@MichaelDeBoey MichaelDeBoey requested a review from machour August 24, 2022 20:20
@changeset-bot
Copy link

changeset-bot bot commented Aug 24, 2022 •
edited
Loading

⚠️ No Changeset found

Latest commit: 084e79f

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

@MichaelDeBoey MichaelDeBoey force-pushed the update-react-18-templates branch 2 times, most recently from 3e2a13e to bf9d54a Compare August 24, 2022 20:38
@MichaelDeBoey MichaelDeBoey mentioned this pull request Aug 24, 2022
Merged
mcansh
mcansh approved these changes Aug 24, 2022
View reviewed changes
Copy link
Collaborator

@mcansh mcansh left a comment

Choose a reason for hiding this comment

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

LGTM, but I'll defer to @jacob-ebey

@MichaelDeBoey MichaelDeBoey assigned jacob-ebey and unassigned mcansh Aug 24, 2022
@MichaelDeBoey MichaelDeBoey force-pushed the update-react-18-templates branch from bf9d54a to 7f0200c Compare August 24, 2022 21:00
@MichaelDeBoey MichaelDeBoey mentioned this pull request Aug 24, 2022
Merged
@MichaelDeBoey MichaelDeBoey force-pushed the update-react-18-templates branch 3 times, most recently from a0866e3 to 5372d7f Compare August 28, 2022 14:28
@MichaelDeBoey MichaelDeBoey force-pushed the update-react-18-templates branch from 5372d7f to 28a68ec Compare September 3, 2022 16:37
@MichaelDeBoey MichaelDeBoey force-pushed the update-react-18-templates branch 2 times, most recently from 165cc00 to d299a81 Compare September 18, 2022 14:13
jacob-ebey
jacob-ebey requested changes Sep 19, 2022
View reviewed changes
templates/express/app/entry.server.tsx Outdated Show resolved Hide resolved
templates/fly/app/entry.server.tsx Outdated Show resolved Hide resolved
templates/remix/app/entry.server.tsx Outdated Show resolved Hide resolved
@MichaelDeBoey MichaelDeBoey force-pushed the update-react-18-templates branch from d299a81 to ffcf220 Compare September 19, 2022 18:25
@MichaelDeBoey MichaelDeBoey force-pushed the update-react-18-templates branch 2 times, most recently from 349611b to dfefb0a Compare September 21, 2022 23:29
mjackson
mjackson requested changes Sep 29, 2022
View reviewed changes
Copy link
Member

@mjackson mjackson left a comment

Choose a reason for hiding this comment

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

Hey @MichaelDeBoey this looks great, thank you. I'd just like a few small syntax changes to remain consistent with the rest of the codebase.

I know I said I could show you how to dedup the code, but I think I prefer it how it is now. It's clear and concise. If people want ternaries, they can make that refactoring themselves.

Thank you! ❤️

templates/express/app/entry.server.tsx
@@ -2,6 +2,7 @@ import { PassThrough } from "stream";
import type { EntryContext } from "@remix-run/node";
import { Response } from "@remix-run/node";
import { RemixServer } from "@remix-run/react";
import isbot from "isbot";
Copy link
Member

Choose a reason for hiding this comment

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

This isn't a huge deal, but TS marks this import as missing in dev. I've always tried to avoid this kind of thing in the past. Not sure the best way to fix it here though since this is in our templates.

Maybe we just add isbot to our monorepo deps to avoid the TS warning? Just a thought. Open to your suggestions.

Screen Shot 2022-09-29 at 3 49 27 PM

Copy link
Member Author

Choose a reason for hiding this comment

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

#4061 will add isbot to the monorepo deps, so these errors should be gone once that one's merged I think.

templates/arc/app/entry.client.tsx Outdated Show resolved Hide resolved
templates/express/app/entry.server.tsx Outdated Show resolved Hide resolved
templates/express/app/entry.server.tsx Outdated Show resolved Hide resolved
templates/arc/app/entry.client.tsx Show resolved Hide resolved
@MichaelDeBoey MichaelDeBoey force-pushed the update-react-18-templates branch from dfefb0a to 3a98dee Compare September 29, 2022 23:38
@MichaelDeBoey MichaelDeBoey force-pushed the update-react-18-templates branch from 3a98dee to 084e79f Compare September 29, 2022 23:39
@MichaelDeBoey MichaelDeBoey merged commit bdec5d4 into remix-run:main Oct 11, 2022
@MichaelDeBoey MichaelDeBoey deleted the update-react-18-templates branch October 11, 2022 17:37
kentcdodds pushed a commit that referenced this pull request Dec 15, 2022
@MichaelDeBoey @kentcdodds
feat(templates): bring React 18 templates in line with indie-stack …
bfae883 
…updates (#4060)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mcansh mcansh mcansh approved these changes

@machour machour Awaiting requested review from machour

@jacob-ebey jacob-ebey Awaiting requested review from jacob-ebey

@mjackson mjackson Awaiting requested review from mjackson

Assignees

@jacob-ebey jacob-ebey

Labels
CLA Signed template:architect template:express template:fly template:netlify template:remix template:vercel
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@MichaelDeBoey @mjackson @mcansh @jacob-ebey