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

templates(classic-compiler/cloudflare-workers): use static assets #10041

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

Conversation

zwily
Copy link

@zwily zwily commented Sep 30, 2024

This PR updates the classic-remix-compiler/cloudflare-workers template to use the new Static Assets feature. It also modernizes the template to match the current vite ones as much as possible. (Includes tailwind by default, uses load-context.ts for augmenting context, etc)

A separate PR (#10034) is open to update the vite template, but this is for the classic compiler. There are a couple reasons to not use vite with cloudflare yet:

  • Your code doesn't run in wrangler, so it doesn't match the execution environment very well.
  • Some bindings don't work through the platform proxy (like Durable Objects).

Once Vite 6 comes out with Vite Environments and your code can run in wrangler, this won't be necessary. But for now it is for at least some of us.

Testing Strategy:

To test, please run create-remix with the template on my fork

npx create-remix@latest --template https://github.com/zwily/remix/tree/workers-assets-classic-compiler/templates/classic-remix-compiler/cloudflare-workers

Then deploy it with wrangler by running:

npm run deploy

Copy link

changeset-bot bot commented Sep 30, 2024

⚠️ No Changeset found

Latest commit: ddf99f6

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

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Sep 30, 2024

Hi @zwily,

Welcome, and thank you for contributing to Remix!

Before we consider your pull request, we ask that you sign our Contributor License Agreement (CLA). We require this only once.

You may review the CLA and sign it by adding your name to contributors.yml.

Once the CLA is signed, the CLA Signed label will be added to the pull request.

If you have already signed the CLA and received this response in error, or if you have any questions, please contact us at [email protected].

Thanks!

- The Remix team

@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Sep 30, 2024

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

@zwily
Copy link
Author

zwily commented Sep 30, 2024

Actually, something is wrong here... When I edit a file in dev and it rebuilds, the local server reloads a ton of times:

⎔ Reloading local server...
⎔ Reloading local server...
⎔ Reloading local server...
⎔ Reloading local server...
⎔ Reloading local server...
⎔ Reloading local server...
⎔ Reloading local server...
⎔ Reloading local server...
⎔ Reloading local server...
⎔ Reloading local server...
⎔ Reloading local server...
⎔ Reloading local server...
⎔ Reloading local server...

I'll try to track that down.

@zwily
Copy link
Author

zwily commented Oct 1, 2024

I have submitted a PR to wrangler to fix the issue:

cloudflare/workers-sdk#6866

@zwily
Copy link
Author

zwily commented Oct 1, 2024

OK, wrangler is fixed and this template is pulling the right version.

Wrangler is also logging every file change on every build which is obnoxious, but not breaking. I have another PR for that here: cloudflare/workers-sdk#6873 but that shouldn't block this.

@zwily zwily force-pushed the workers-assets-classic-compiler branch from 4aeca3a to 1383e86 Compare October 4, 2024 16:13
@zwily
Copy link
Author

zwily commented Oct 4, 2024

Okay, this template is in working shape now with current wrangler.

(The only thing is we have --x-dev-env disabled for now because of this issue in wrangler which is closed but not fixed: cloudflare/workers-sdk#6876)

@MichaelDeBoey
Copy link
Member

@zwily It seems the mentioned bug is now fixed?

Putting the "npm run build" command in wrangler.toml was causing wrangler to rebuild
the app when it started, which is not necessary because remix itself handles the build.
Instead we have wrangler not run the build, and run `npm run build` before deploy instead.
There's an ongoing issue with multiple reloads with the new x-dev-env,
so we're turning it off for now:

cloudflare/workers-sdk#6876
We can't bundle "cloudflare:workers", but that will be imported when
writing a DurableObject with RPC.
@MichaelDeBoey MichaelDeBoey force-pushed the workers-assets-classic-compiler branch from 67a99bd to ddf99f6 Compare October 9, 2024 21:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants