template: v2 dev

[jacob-ebey]

Closes: #

• Docs
• Tests

Testing Strategy:

[changeset-bot]

⚠️ No Changeset found

Latest commit: 3f0ba37

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

[huw]

I think this should be ["workerd", "worker", "browser"]:

• workerd is the new standard for Cloudflare Workers and could have slightly different code, usually optimised for edge runtimes. This is being adopted by React with a different bundle from their browser bundle.
• worker should remain, because it’ll usually work and rely on less code than the browser bundle.
• browser lets us fall back to browser code in packages that don’t have one of the other builds. Anecdotally, some of my code breaks without adding this condition. I’d argue this is more likely to happen than the inverse (a user imports a code that relies on browser-specific globals that Workers doesn’t have).

[huw]

I don’t think this line will work with serverDependenciesToBundle: "all", unless something has changed recently. The bundler should be complaining about being unable to find this dependency. If that’s the case, the solution is to use serverDependenciesToBundle: [/^(?!(__STATIC_CONTENT_MANIFEST)$).*$/u], which is hacky but at least avoids the issue.

[huw]

Using __STATIC_CONTENT_MANIFEST on the environment like this is a Miniflare / Wrangler Local bug. It will break when using Wrangler in live mode or in production. You have to import it like so, and add serverDependenciesToBundle: [/^(?!(__STATIC_CONTENT_MANIFEST)$).*$/u] to your remix.config.js.

[acusti]

i tried using the server.ts from this template with the latest versions of remix and wrangler and found that it broke my static assets in production. so i tried reverting this from using env.__STATIC_CONTENT_MANIFEST to using

import assetManifestAsString from '__STATIC_CONTENT_MANIFEST';
const assetManifest = JSON.parse(assetManifestAsString);

and passing that as ASSET_MANIFEST to getAssetFromKV() (as well as updating serverDependenciesToBundle to exclude __STATIC_CONTENT_MANIFEST), and it seemed to fix the css files that were 404ing but my /build/root-{hash}.js and /build/manifest-{hash}.js still 404ed in production. so i reverted server.ts back to:

import { createEventHandler } from "@remix-run/cloudflare-workers";
import * as build from "@remix-run/dev/server-build";

addEventListener(
  "fetch",
  createEventHandler({ build, mode: process.env.NODE_ENV })
);

and production is fixed (but no HMR in dev, obviously).

[appden]

This logs too soon for me resulting in a failed request from the browser after HMR updates because the server is not yet listening on the port when it's restarted by wrangler. By adding a setTimeout things work fine but definitely is hacky (I'm on 100ms to be safe).

[huw]

Per my comment on #6415, I think this is only a problem in Wrangler v2. The appropriate fix for this should probably be to upgrade the dependency to v3.

I think this is likely a problem for the Workers template too, but I haven’t tested with Wrangler v2 on my setup.

[appden]

You're right, it works great without setTimeout with Wrangler v3, so definitely recommend updating the dev dependency for the templates. Thanks!

[pcattori]

Superceded by #6650