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.

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

feat: add support for Vite #137

Merged
merged 6 commits into from
Feb 22, 2024
Merged

feat: add support for Vite #137

merged 6 commits into from
Feb 22, 2024

Conversation

ascorbic
Copy link
Contributor

@ascorbic ascorbic commented Feb 21, 2024

Description

Updates the template to use Vite. This is based on the default Remix Vite template. There's far less mucking about needed, so I've simplified the init script a lot.

@ascorbic ascorbic requested a review from a team as a code owner February 21, 2024 13:16
Copy link

netlify bot commented Feb 21, 2024

Deploy Preview for remix-edge-on ready!

Name Link
🔨 Latest commit
🔍 Latest deploy log https://app.netlify.com/sites/remix-edge-on/deploys/65d71a4371c15588bdaf910a
😎 Deploy Preview https://deploy-preview-137--remix-edge-on.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Feb 21, 2024

Deploy Preview for remix-on ready!

Name Link
🔨 Latest commit
🔍 Latest deploy log https://app.netlify.com/sites/remix-on/deploys/65d719f1b1e8ef894c808e7c
😎 Deploy Preview https://deploy-preview-137--remix-on.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@github-actions github-actions bot added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Feb 21, 2024
@ascorbic ascorbic marked this pull request as draft February 21, 2024 13:17
@ascorbic ascorbic marked this pull request as ready for review February 21, 2024 17:36
package.json Show resolved Hide resolved
Comment on lines 141 to 143
if (useEdge) {
await copyTemplateFiles(rootDirectory);
}
Copy link
Contributor

Choose a reason for hiding this comment

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

somewhat NIT, but this setup with edge overwriting "default" lambda/function is a bit hard to track (what's different between them, what's common etc)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, I did consider handling all lambda-specific files in the same way as edge, but in the end I wanted to ensure the template would at least build if init hadn't run. An alternative might be to set the "build" command to "remix init", and have that overwrite the command. In fact, I think I'll do that.

pieh
pieh previously approved these changes Feb 22, 2024
package.json Show resolved Hide resolved

// Remove the auto-init command from the scripts
for (const script of ["build", "start", "dev"]) {
scripts[script] = scripts[script]?.replace("remix init && ", "");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pieh here

Copy link
Contributor

Choose a reason for hiding this comment

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

should we have concerns for mutating package.json and possibly causing surprising modifications and git tracking?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We need to mutate it anyway, because they each have different dependencies. In almost all cases this is run as part of the template setup from create-remix, which commits the changed files. Even in the case where it's run from the package.json command there'll still be a stack of modifications anyway because we're moving all the files around to setup the template.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was mostly thinking about cases when it wouldn't run at init for some reason (I did assume that template init does handle commiting initial modifications), but in this case there's not much for us to do, possibly than try to recognize it's not init execution and maybe print message / try to commit ourselves (but this is can of worms and probably leaving user with modified files and letting them handle it is better than trying to handle that ourselves)

@kodiakhq kodiakhq bot merged commit 65f7e25 into main Feb 22, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge type: feature code contributing to the implementation of a feature and/or user facing functionality
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants