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

Compatibility with remix 2.x #62

Open
acusti opened this issue Feb 8, 2024 · 7 comments
Open

Compatibility with remix 2.x #62

acusti opened this issue Feb 8, 2024 · 7 comments

Comments

@acusti
Copy link
Contributor

acusti commented Feb 8, 2024

i’m very impressed by this project and was quite excited to get started with the @superflare/remix package. however, i see that it’s depending on remix version ^1.14.1, whereas i’m on the latest version of remix (v2.6.0 as of now). are there any plans to migrate the remix package to the v2.x branch of remix? i have no idea how deep the integration is (and how much is likely to have broken between 1.14.x and 2.x), but i’d also be happy to take a look at making a PR to get the package working with 2.x. @jplhomer would that be something you would be interested in and able to devote time to code review / merging it upstream?

thanks for this project! i hope superflare is going to continue to be a viable option for enhancing the cloudflare workers / remix stack, because i think it’s currently an unbeatable stack in terms of performance, cost, and developer productivity.

@jplhomer
Copy link
Owner

jplhomer commented Feb 8, 2024

Hi @acusti thanks for the note. I would love to upgrade to Remix v2 as well as the new Cloudflare Vite setup. I haven't looked into this yet, but you're welcome to do so and let me know how it goes!

The biggest hangup will likely be around the ability to control the entire request lifecycle. Superflare relies on setting singleton values to reference registered models, events and the like — and if the framework doesn't allow us to hook into that, it makes it hard to be effective.

I believe the Remix Vite plugin supports Workers Pages only, while Superflare was built for Workers Sites due to lack of support for configuration in Pages (this may have changed).

Another challenge is that Cloudflare is actively migrating to a single Workers product instead of split behavior between Workers Sites and Workers Pages. This makes it challenging to build a nice onboarding experience for Superflare users on top of an ever-changing foundation. Part of me wants to wait until that migration has stabilized, and until Workers Pages has full compatibility with Workers Sites, before doing an overhaul of Superflare.

Anyway, just off-the-cuff thoughts. Let me know what you think!

@Hades32
Copy link

Hades32 commented Feb 11, 2024

The last time I asked them they had no ETA on when the merge of workers and pages would be complete. I wouldn't hold my breath tbh. From what I can see it would make sense to go all-in on pages at that seems to be most integrated experience so far

@acusti
Copy link
Contributor Author

acusti commented Feb 12, 2024

@jplhomer i’m also using the remix cloudflare workers integration (as opposed to pages), but i guess i’ll have to bite the bullet soon and migrate to pages. and yes, you’re correct that the initial support for vite is pages-only, at least for the time-being.

regarding lack of support for configuration in pages, it seems like wrangler v3.24 added full support for bindings in wrangler.toml for cloudflare pages via a new getBindingsProxy utility.

if it will be a long time before the merge of workers and pages is complete (as per @Hades32’s comment), i suppose it’s worth embarking on this effort, though i’m somewhat intimidated at the prospect of handling a number of migrations simultaneously (remix v1 → v2, esbuild → vite, and workers → pages). because i’m using workers and because i’m on the latest version of remix without adopting vite and i don’t really have complaints, i’m selfishly tempted to just work on the remix v1 → v2 migration, but the recent comments signaling no more support for workers means maybe the more important initial migration would be from workers → pages. but that makes the end result i desire, which is being able to use superflare in my web app, seem further away.

@jplhomer which feels like a higher priority to you?

@jplhomer
Copy link
Owner

regarding lack of support for configuration in pages, it seems like wrangler v3.24 added full support for bindings in wrangler.toml for cloudflare pages via a new getBindingsProxy utility.

Oh very cool!

I guess I'd personally see what it's like to attempt to use Superflare in Remix v2. If there are major blocking things (because of the switch to the provider support for Pages only), then we probably need to switch to focus on that first. But let's see how far we can get!

@acusti
Copy link
Contributor Author

acusti commented Jul 13, 2024

quick update that i’m finally taking this on now. i recently successfully upgraded my remix v2 cloudflare workers app from the classic compiler to the vite compiler (based on the updated cloudflare workers template), so my hope is to migrate superflare to run on wrangler v3 and vite, while avoiding the cloudflare workers → pages part of the equation.

quick notes: i couldn’t get better-sqlite3 to install on my M1 MacBook Pro running macOS Sonoma 14.5 with any version of pnpm. i wound up upgrading pnpm to v9.5.0 (latest as of now) and better-sqlite3 to v11.1.2 (latest) to get the installation to succeed (i was running into errors with the better-sqlite3 install script and node-gyp rebuild --release).

@jplhomer let me know if anything about the above is concerning or seems like the wrong approach. one nice thing about going from esbuild to vite is that means no more need to patch remix in order to modify the esbuild config to enable nodejs_compat for node built-ins.

@acusti
Copy link
Contributor Author

acusti commented Jul 14, 2024

looking at wrangler v2 → v3 changes, it seems like the wrangler d1 migrations apply command no longer supports the -j option. i couldn’t find documentation on what that option did in wrangler v2, but in wrangler v3, the docs specify that “This command will prompt you to confirm the migrations you are about to apply. Confirm that you would like to proceed. After, a backup will be captured.”

in order to make that work, it seems to me like runWranglerCommand will have to be updated to support detecting CLI prompts and replying with Y automatically in order to preserve the current wranglerMigrate behavior. the AWS amplify CLI uses a --yes flag for that behavior, so i suppose that could be one way to enable that behavior in runWranglerCommand, or it could just always confirm any confirmations when they arise when running a command.


i’ve also been looking at upgrading from miniflare v2 → v3, and the APIs look wildly different, with an overall substantial reduction in available API surface area. looking at the miniflare v3 docs, the entirety of the D1 API, for example, seems to be:

const mf = new Miniflare({
  d1Databases: {
    DB: "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx"
  },
});

const db = await mf.getD1Database("DB");
const { results } = await db.prepare("<Query>");

console.log(await res.json(results));

which makes me think that a way to replace the createSQLiteDB method being used for the superflare CLI’s seed and migrate commands could mean going from

const db = await createSQLiteDB(dbPath, logger.log);

to

const mf = new Miniflare({
  d1Databases: { [dbPath]: "xxxxxxxx-xxxx-xxxx-xxxx-xxxxxxxxxxxx" },
});
const db = await mf.getD1Database(dbPath, logger.log);

might do it, but that raises the question of where the database ID comes from to pass as part of the d1Databases option, and whether there should be some sort of shared miniflare client instance reused across superflare.

anywho, there’s a lot i’m still familiarizing myself, but i figured i’d document my questions as i go. i can also create a draft PR to start a conversation there if that would be more helpful.

@jplhomer
Copy link
Owner

@acusti This is great stuff! Thanks for diving into this. I've started and stopped at various attempts myself, so I appreciate the follow-through.

Regarding PNPM and better-sqlite: yeah, it's a major pain in the butt. I think upgrading to latest seems to fix everything (but probably breaks other things... what can you do).

The wrangler updates are probably the biggest challenge in upgrading to the latest versions, as you've discovered. Now that miniflare defers everything to workerd, there's a lot less manual tooling involved. I know the wrangler functionality continues to improve as well, so I think it would be a smart strategy to keep superflare CLI as light of a wrapper as possible. Sorry, I haven't dug into that space in a bit or I would have more tips to offer :)

Feel free to open PRs and push discussion over there, too!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants