Migrate superflare and @superflare/remix from remix v2 → react-router v7#75
Open
acusti wants to merge 29 commits intojplhomer:mainfrom
Open
Migrate superflare and @superflare/remix from remix v2 → react-router v7#75acusti wants to merge 29 commits intojplhomer:mainfrom
acusti wants to merge 29 commits intojplhomer:mainfrom
Conversation
follow-up to 905ba4c
using serialize skips the potential toJSON() override used to e.g. not include User.password in the serialized form of a User when it is serialized as a relation of another model (e.g. Article in examples/remix-cms)
allows for e.g.:
const articles = await Article.with("user").orderBy("createdAt", "desc").toJSON();
from a loader to explicitly return the serialized version of a model to provide it to a component. this makes it so that the version of the model that is provided during SSR is exactly equivalent to the version of the model that is made available to the component when rendered on the client. otherwise, the data made available to the component during SSR when using single fetch is the actual model instance.
single fetch uses turbo-stream to serialize data, meaning that model.toJSON() is no longer automatically invoked as was the case with the remix json() helper. turbo-stream’s encode function calls flatten, which defines a partsForObj util that gets called for POJOs and uses Object.keys(): https://github.com/jacob-ebey/turbo-stream/blob/main/src/flatten.ts#L50 customizing ownKeys() to use toJSON() to return the appropriate keys ensures that the object gets serialized by turbo-stream in the form defined by the model ( including any customizations, e.g. the default User model, which removes the password field from the result) however, calling target.toJSON() means that relation field keys are also returned, which causes the relation model name to exist on the model (though undefined). this is why we need to check in the get() proxy trap if the prop is not undefined on target, rather than just check for the existence of the prop in target.
• add an IsSingle type argument to QueryBuilder to track if first() has been invoked • add the toJSON method to QueryBuilder • update return types of BaseModel query methods to return a QueryBuilder • update QueryBuilder promise methods (then/catch) to resolve query results based on if IsSingle (i.e. if first() has been invoked) or not
commit: |
Owner
|
Sweet! Could we potentially use https://github.com/mjackson/remix-the-web/tree/main/packages/node-fetch-server#low-level-api for the |
this allows proper type checking of resolved instances of the model when using single fetch
the placeholder GITHUB_TOKEN in .dev.vars ensures that wrangler includes it in its typegen (worker-configuration.d.ts)
acusti
force-pushed
the
rr7
branch
2 times, most recently
from
b543b8b to
063f83a
Compare
Contributor
Author
|
@jplhomer good call! i switched over to |
this allows proper type checking of resolved instances of the model when using single fetch
this required duplicating node-adapter.js and maintaining it within superflare-remix, because it’s no longer included in the published package’s dist/ directory. here’s a PR that exposes the (from|to)NodeRequest utilities that we depend on in the superflareDevProxyVitePlugin: remix-run/react-router#12774 if it gets merged, we can remove superflare-remix/node.adapter.ts and import those utils alongside cloudflareDevProxy note that i also needed to update superflare-remix’s tsconfig.json module and moduleResolution settings to get it to build without error (presumably due to changes between the published remix vs react-router packages
fixes 4 type errors in examples/remix-cms
ensures that the seed command can complete all of its async work and exit properly and makes it so that errors can be caught and logged (lines 45–47)
• update entry.server.tsx based on: https://github.com/remix-run/react-router-templates/blob/main/cloudflare/app/entry.server.tsx • remove entry.client.tsx files (they aren’t in the template examples any more)
new versions of wrangler include a new _cf_METADATA table that needs to be filtered out alongside _cf_KV, which i extrapolate to mean that any internal cloudflare tables introduced in the future will start with the _cf_ prefix
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
this also meant migrating
examples/remix-cmsfrom the v1 file routing style to the v2 flat routing style (1296744) and removing the deprecated@remix-run/eslint-configpackage fromapps/siteandexamples/remix-cms.the commit that migrates superflare and @superflare/remix is baca18b. doing so required duplicating @react-router/dev’s
node-adapter.jsand maintaining it within superflare-remix, because it’s no longer included in the published package’sdist/directory. i also made a PR in react-router that exposes the(from|to)NodeRequestutilities that we depend on in thesuperflareDevProxyVitePluginand would remove the need for duplicating that file in superflare. if it gets merged, we can removesuperflare-remix/node.adapter.tsand import those utils alongsidecloudflareDevProxy. note that i also needed to update superflare-remix’stsconfig.jsonmoduleandmoduleResolutionsettings to get it to build without error (presumably due to changes between the published remix vs react-router packages).