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(cloudflare-workers): convert main entry file from JS → TS #9774

Merged
merged 4 commits into from
Jul 26, 2024

Conversation

acusti
Copy link
Contributor

@acusti acusti commented Jul 18, 2024

follow-up to #9345

This enables type checking of the main workers entry file in the cloudflare workers template by converting it from .js to .ts and by fixing an incompatibility with the AppLoadContext type that is surfaced by the new type checking. Note that I also renamed server.jsworker.ts, but I’m happy to rename it back to server.ts if that is preferred. Mainly, I just saw it as an opportunity to slightly simplify the migration from the classic compiler cloudflare worker setup to the vite-based one. Lastly, I removed the redundant SCRIPT argument from the wrangler dev command (it’s already defined as the main file via wrangler.toml; reference here). Lastly, I added // eslint-disable-line import/no-unresolved to import * as build from "./build/server"; because it was causing a lint error from the CI lint check, and I added // @ts-ignore This file won’t exist if it hasn’t yet been built above it because running tsc before building the app in my CI was causing a “Cannot Find Module” error.

Testing Strategy:

I converted the main file of my remix cloudflare workers app from .js to .ts and resolved the resulting typescript errors that were produced. All the changes are types-only, so there is no effect on runtime behavior.

Copy link

changeset-bot bot commented Jul 18, 2024

⚠️ No Changeset found

Latest commit: d446ab6

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

@acusti acusti force-pushed the patch-2 branch 4 times, most recently from 07ba7c1 to 7bc2e05 Compare July 19, 2024 20:00
acusti added a commit to acusti/superflare that referenced this pull request Jul 19, 2024
acusti added a commit to acusti/superflare that referenced this pull request Jul 22, 2024
Copy link
Member

@markdalgleish markdalgleish left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

Just a heads up that I've rolled the worker.ts rename back to server.ts mainly just to keep it in line with the old template.

@markdalgleish markdalgleish merged commit 8c146c5 into remix-run:main Jul 26, 2024
2 checks passed
@MichaelDeBoey MichaelDeBoey changed the title Convert cloudflare workers template’s main entry file from JS → TS templates(cloudflare-workers): convert main entry file from JS → TS Jul 26, 2024
@acusti acusti deleted the patch-2 branch July 26, 2024 16:12
jplhomer added a commit to jplhomer/superflare that referenced this pull request Nov 15, 2024
…#66)

* ⬆️ Upgrade pnpm/wrangler/better-sqlite3

plus remove the @remix-run/dev patch

* Remove defunct NO_D1_WARNING env var

* ⬆️ Upgrade to latest remix v2.x

* ⬆️ Upgrade to latest typescript (v5.x)

* Remove defunct custom pnpm resolutions

* Update remix vite cloudflare-workers dependencies

• reference: https://github.com/remix-run/remix/tree/main/templates/cloudflare-workers
• also, update @superflare/remix peerDependencies
• remove unused @remix-run/serve dependency
• move @remix-run/server-runtime from devDependencies → dependencies
• upgrade to latest @cloudflare/workers-types + make it consistent across workspaces

* Upgrade to latest tailwind + markdoc

* Update docs site to work w/ remix v2 + vite (WIP)

* Update remix-cms example for remix v2 + vite (WIP)

* Update remix template for remix v2 + vite (WIP)

* Make generateTypesFromSqlite take D1Database db

…and refactor it to be an async function, plus add a D1DatabaseAdaptor for exposing a compatible API for a SqlliteDB instance

* Fix types via generics + conditional in Button.tsx

* Handle rename LoaderArgs → LoaderFunctionArgs

* Handle rename ActionArgs → ActionFunctionArgs

* Make meta functions remix v2 compatible

…plus take advantage of MetaFunction type’s generics

* Remove deprecated (now default) wrangler CLI flags

* 📝 Update docs site README env var info

* Regenerate docs site worker types with .dev.vars

* Fix types in docs site’s docs.server.ts

* Update docs site’s env var names in .env.example

* Adapt docs site’s loaders to new ctx shape

* Remove redundant wrangler dev script arg

From the wrangler docs:
“Only required if your wrangler.toml does not include a main key”
Source:
https://developers.cloudflare.com/workers/wrangler/commands/#dev

* Restore TS cloudflare worker’s entry file + name

reproduces the changes suggested in remix-run/remix#9774

* Remove defunct cloudflare Env type declarations

• the Env type is now generated by the wrangler types command and written to worker-configuration.d.ts
• note however that it doesn’t include CF_PAGES or APP_KEY, which are both used in examples/remix-cms/functions/[[remix]].ts

* Restore focus-visible polyfill

* 📝 Fix a typo in superflare/cli/new.ts

* Import + use types from @cloudflare/workers-types

* Adapt superflare’s loadContext to getPlatformProxy

* Add auth + session to AppLoadContext type

* Restore superflare handlers in remix template

* 📝 Update docs based on latest APIs

* Fix package.json types location for TS v.4.7+

* Add typecheck run script to packages/*

* Add missing @cloudflare/workers-types devDep

* ⬆️ Upgrade @cloudflare/workers-types

* Restore ambient workers types + original filenames

• no need to rename cloudlfare.env.d.ts → env.d.ts
• i kept the new env.d.ts name for the remix template in order to match the setup of the “official” cloudflare workers remix template (reference: https://github.com/remix-run/remix/tree/main/templates/cloudflare-workers)

* Disambiguate workers Request type (from global)

* Fix superflare build by making redis pkgs external

* ⬆️ Upgrade latest eslint-plugin-turbo

* Use createTestDatabase in /d1-types.test.ts

this means that superflare/cli/d1-database.ts’ createSQLiteDB util is no longer used anywhere

* Specify latest version for @miniflare/* deps

* Migrate from local /d1js → @miniflare/d1

* Add .dev.vars to .gitignore

* Remove global process type from examples/remix-cms

* Use createD1Database util in migrateHandler

* Use .dev.vars to define APP_KEY + regen types

* Simplify secure protocol check for session cookie

* Fix reference to remix build in remix-cms worker

* Replace non-existent Auth import → SuperflareAuth

* ⬆️ Upgrade to latest vite (v5.3.4)

* ⬆️ Upgrade latest docs site UI libs

the prism-react-renderer upgrade includes some breaking changes, so Fence.tsx and Hero.tsx have been updated to work with those changes as well

* Add ssr.noExternal to fix docs site’s build issues

based on algolia/docsearch#2259

* Restore remix-cms DO Channel export to main

* 🚿 Cleanup unneeded TS config values

* Restore queue + scheduled to remix-cms/worker.ts

* Fix naming of docs site’s root route’s index route

https://remix.run/docs/en/main/file-conventions/routes#basic-routes

* Filter out "_cf_KV" table when handling D1 models

* 🚿 Remove need for intermediate foo var

* Crane + file cabinet emojis need an extra space

🤷

* Replace better-sqlite3 → @miniflare/d1 in tests

this means no more direct dependency on better-sqlite3

* 🚿 Remove direct deps on better-sqlite3

* 🚿 Remove unused generate migration option

* Add getD1Database(dbName) using getPlatformProxy

* Refactor getD1Database from wrangler → miniflare@3

• the CLI migrate command doesn’t work if miniflare is imported statically
• adding it as a dependency ensures we get the correct types for miniflare v3.x (otherwise we get miniflare v2.x types)
• the previous version (using wrangler’s getPlatformProxy) was resulting in the following error:

service core:user:__WRANGLER_EXTERNAL_DURABLE_OBJECTS_WORKER: Worker "core:user:__WRANGLER_EXTERNAL_DURABLE_OBJECTS_WORKER"'s binding "Channel" refers to a service "core:user:worker", but no such service is defined.

* Refactor d1 commands from dbPath → dbName

• also refactor from using createD1Database → getD1Database
• refactor migrate --fresh option from using fs.rm() to delete the existing database file to dropping all user-facing tables tables from the existing database

* Explicitly exit CLI migrate command

* Update workers compatibility_date to latest

https://developers.cloudflare.com/workers/configuration/compatibility-dates/#properly-extract-blob-mime-type-from-content-type-headers

* Remove defunct <LiveReload> component

reference: https://remix.run/docs/en/main/guides/vite#hmr--hdr

* Remove inapplicable dev:superflare command

* Use vite’s built-in CSS handling in docs site

* Use CJS-compatible pluralize import

* Use remix vite:dev for superflare dev command

preserved the “Using (D1|R2) bindings: …” console logging and updated it to always print (not just in pages mode), because the remix vite:dev command doesn’t provide any of that info to the user

* Use superflare dev command for dev run script

* ⬆️ Upgrade to latest @clack/prompts

* Remove redundant ellipsis from spinner text

the spinner already animates a repeating ellipsis onto the end of the text as it spins, so having the text end with “...” is redundant

* Shorten spinner text to avoid text wrap bug

the bug: bombshell-dev/clack#132

* Remove defunct --legacy-peer-deps

all of the remix cloudflare packages have been upgraded to @cloudlfare/workers-types v4 (e.g. https://github.com/remix-run/remix/blob/main/packages/remix-cloudflare/package.json)

* Tighten up headers whitespace for readability

* Add snapshot publishing

* No compacting

* Allow `--repo` to be passed to `superflare new`

* feat: add support for account selection during `superflare new`

* Add getLoadContext to @superflare/remix

* Add superflareDevProxyVitePlugin for convenience

* Cast-to-any as type incompatibility workaround

* Use superflareDevProxyVitePlugin in vite.config

* Remove duplicate local load-context.ts

already supplied by @superflare/remix

* 🚿 Remove unused variable

* Add experimentalJsonConfig for wrangler.json

enables wrangler to pick up the bindings and provide them from getPlatformProxy

* Use getLoadContext to instantiate config singleton

getLoadContext is the common function invoked when handling requests across all environments (dev with vite and prod with workers)

* Use relative import to work pre-tsconfig

the superflare.config.ts is imported into the vite.config.ts, so the imports have to be resolved before the vite-tsconfig-paths plugin has been enabled, so `~/…` imports don’t work

* Resolve type error by updating config type

* Update types to resolve no implicit any errors

• HasMany’s save argument looks like Model[] | Model to me
• HasMany’s create argument is any, so i made that explicit in the map()

* Add script_name for Channel DO binding

wrangler’s getPlatformProxy requires the script_name in order to be able to proxy any Durable Object bindings: https://developers.cloudflare.com/workers/wrangler/api/#supported-bindings

* Clarify comments + use implicit return

* Differentiate index.types.ts type exports

* Replace type DefineConfigResult → DefineConfigReturn

allows us to only import the DefineConfigReturn type instead of importing the actual defineConfig function and using ReturnType<typeof defineConfig<Env>>

* Fix superflare-remix vite plugin + getLoadContext

• calling getLoadContext from a vite plugin means that it doesn’t execute in the same context as the actual application, so it gets its own Config class singleton
• update getLoadContext to take SuperflareAuth and SuperflareSession, then use viteDevServer.ssrLoadModule from the vite plugin to load both classes and provide them to getLoadContext
• use viteDevServer.ssrLoadModule from the vite plugin to load superflare.config.ts and provide it to getLoadContext

* Update vite.configs to not pass superflare.config

* Restore ~/ import in superflare.config.ts

the fixed implementation for initializing the config in getLoadContext means that the config file is now imported by vite and so the import alias now works

* Use entry.(client|server).tsx from remix example

sources:
https://github.com/remix-run/remix/blob/main/templates/cloudflare-workers/app/entry.client.tsx
https://github.com/remix-run/remix/blob/main/templates/cloudflare-workers/app/entry.server.tsx

* 📦 Update postcss + autoprefixer deps

* Fix tailwind styles in examples/remix-cms

* Fix input width on mobile + add a max width

• they were defaulting to .col-span-1 below 640px
• now they span across the full row at all breakpoints but with a 24rem (384px) max-width

* Add props.autoComplete to inputs + typeof action

* Fix getSessionCookie, consistent superflare import

adds getSessionCookie to AppLoadContext and provides it from load-context.ts

* Move config init to superflareDevProxyVitePlugin

also, bring over code to ensure session has a sessionId from superflare#handleFetch

* Ensure type-only superflare import + remove unused

* Add commitSession logic to entry.server.tsx

* Use wrangler’s getPlatformProxy in getD1Database

• reverts the changes implemented in 22f47d4
• the reason getPlatformProxy didn’t work when implemented in fe766b9 is that getPlatformProxy cannot handle Durable Objects if a script_name isn’t provided for the bindings (which was resolved in f5874c0)

* Refactor @superflare/remix types to take Env

• move @remix-run/cloudflare AppLoadContext definition to index.ts
• use AppLoadContext where necessary
• use WorkersRequest for accuracy + global Request for @remix-run/cloudflare’s createRequestHandler return value compatibility
• restore local ./load-context.ts to expand @remix-run/cloudflare’s AppLoadContext type to include a cloudflare property based on Env interface from ./worker-configuration.d.ts

* Remove defunct "superflare" import

* Fix import TextareaMarkdown for vite

related: vitejs/vite#2139 (comment)

* Fix types for routes/auth/hooks.ts’ useAdmin

* Upgrade isbot + eslint-(config|plugin)-turbo

* 🚿 Remove defunct concurrently deps

* 📦 Upgrade to latest concurrently

* Use experimental json config for wrangler deploy

* 🚿 Cleanup unused dependencies

* Export getLoadContext from @superflare/remix

* Move superflareDevProxyVitePlugin → remix-dev pkg

added new @superflare/remix-dev package to host the superflareDevProxyVitePlugin in such a way that it can be independently imported into users’ vite.config.ts file without then getting included in the built cloudflare workers bundle

* Use @superflare/remix-dev in remix-cms + template

the template dependency string will have to be changed from "workspace:*" → "*" once the new package has been published

* Update superflare dev to also run wrangler dev

fixes durable objects in dev: https://developers.cloudflare.com/workers/wrangler/api/#supported-bindings

* Run build serially (superflare/remix depends on superflare)

* Remove unused dependency from apps/site/

* Fix tsconfig’s cloudflare.env.d.ts include path

* Move vite dev plugin to @superflare/remix/dev

previously available as @superflare/remix-dev, but this is simpler

* Use @superflare/remix/dev in examples

* Use installed wrangler version (not latest) in CLI

* Use static assets for workers feature

* Remove defunct DBConfig type

* Fix d1_databases wrangler config field name

* Add script_name for Channel DO binding in CLI

same fix as f5874c0 but applied to the superflare CLI new command

wrangler’s getPlatformProxy requires the script_name in order to be able to proxy any Durable Object bindings: https://developers.cloudflare.com/workers/wrangler/api/#supported-bindings

* Adopt default remix v2 flat routing convention

https://remix.run/docs/en/main/file-conventions/routes

* Add generic types to template useActionData

* Drop migrations table for migrate --fresh option

* Don’t return handleQueue Promise.all

• the return value will be an array of undefined values of the same length as batch.messages, which doesn’t seem useful
* returning that array leads to a type error in the main worker file due to incompatibility with the return value of the ExportedHandlerQueueHandler<Env, unknown> type

* Fix request object type in handleFetch

* Fix  type of scheduled event argument

reference: https://github.com/cloudflare/workers-sdk/blob/main/packages/wrangler/templates/new-worker-scheduled.ts#L26

* DO script_name is worker app name (not file name)

follow-up to ebdbc99

* Fix script_name for DO binding in remix-cms example

• follow-up to f5874c0
• turns out the script_name should be the worker’s app name (not its file name)

* Separate @superflare/remix/dev completely from @superflare/remix

removing the ;./index' import from superflare-remix/dev.ts makes it so that there is no possible issue with using static `import {…} from 'superflare'` in superflare-remix/index.ts, because that file won’t be executed outside of the vite bundle/execution context

* Specify a wrangler version that supports static assets

* Fix superflare dev command + suppress wrangler dev io

* Add @remix-run/server-runtime, use latest pnpm

* Specify minimum remix version for devProxyVite plugin

v2.12.1 includes remix-run/remix#9976, which changed the API contract of the fromNodeRequest util that we will depend on in superflareDevProxyVitePlugin

* Fix committing session changes on response in dev

using entry.server.tsx meant that sessions weren’t committed for redirects, e.g. the default logout route action, which calls
```
auth.logout();
return redirect("/");
```
this commit takes over the middleware portion of the cloudflareDevProxyVitePlugin logic and frames the request/response lifecycle with superflare.config initialization + getLoadContext before the request handler is invoked, and commit session cookie handling after

* Ensure <meta charset> is first thing rendered

fixes UTF 8 text encoding / rendering, at least when running the docs app locally

* Include “.md” ext in default docs filepath

* Upgrade to latest version of turbo (2.x)

* 🚿 Strip down entry.server.tsx files

* Add lang to all code blocks for prism-react-renderer

not having a language identifier results in:

TypeError: Cannot read properties of undefined (reading 'toLowerCase')
    at Highlight (/Users/foo/superflare/node_modules/.pnpm/[email protected][email protected]/node_modules/prism-react-renderer/src/components/highlight.ts:14:30)

* Fix server-runtime imports (should be cloudflare)

* Add better-sqlite3 devDependency in packages/superflare

fixes tests locally on my machine

* Make getPlatformProxy experimentalJsonConfig: true

also, allow it to be overwritten in case someone wants to make superflare work with a wrangler.toml

* Add proper migrations to Remix CMS

---------

Co-authored-by: Josh Larson <[email protected]>
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.

4 participants