zth / rescript-relay-router Goto Github PK
View Code? Open in Web Editor NEWLicense: MIT License
License: MIT License
We have a heuristic that only does a full route update when path params change. This is to make it easy to do more "surgical" updates of query params, where only what you're interested in is fetched/refetched. But, this becomes confusing because it means using links that just change query params does not actually work unless they also change at least some part of the path. This goes against the goal of advocating for thinking about "params" rather than path vs query params.
I think we can support both cases quite easily, so let's find a good way to do that, maintaining "links always work" while also having the option to do more surgical updates to query params that does not trigger a full route re-render.
cc @Kingdutch , we should also cover the "click on the same link and have it refresh the current route" case here.
We can validate at build time that the patterns we piece together from the route structure will actually work, by simply matching on them via react-router as we extract them. Do that, and add errors if they fail.
Follow-up to #86
This is the biggest size culprit. Find a way to reduce its size.
We have some code which deals with Component
loading, which makes sense because it originates from lazy loading components. However, with our new and improved set-up there's nothing that mandates it's specifically a Component
and we're more dealing with Scripts
's where the underlying preloading of components already keeps track of what scripts are needed.
Update the API to refer to Script
instead of Component
so it's clearer for consumers that this is generic and not specialized.
Logic exists in both the RescriptRelayVitePlugin and in lookup.mjs
. Figure out why and dry up.
Syntax error!
/<project>/src/routes/__generated__/RouteDeclarations.res:525:43-46
523 ┆ prepared: prepared,
524 ┆
525 ┆ type: preparedProps.type,
526 ┆ direction: preparedProps.direction,
527 ┆ at: preparedProps.at,
`type` is a reserved keyword. Keywords need to be escaped: \"type"
JSONC allow us to add comments, which is great for our purposes. Should document that.
This ensures that applications no longer need to define it themselves.
The bindings should also move into the router, but the difficulty there is that they rely on a stream type which is currently tied to NodeJs.
We should check if the stream also works for things like Cloudflare workers (or other serverless environments) and either a) provide a TransformStream (an example of this vs the NodeJs writable stream can be found in the React 18 working group discussions) b) if Writable streams are supported on those platforms too we could use the same class but have to make the type abstract
Fix #84
It's easy to create a fallback page for when a page is not found. However, the result of this is that when this catch-all route is matched the status code is still 200 rather than 404. Especially when a browser tries to load some kind of asset this may be problematic.
Follow-up to #86
Extend the types of things that we can preload. MDN contains a list of valid values for as that we probably want to try out with e.g. Vite to see how it exposes the data to ensure we can get it into our manifest and preload it with the right time. We may decide that we want to replace the empty arrays (easy to use) with nullable objects (smaller to transfer) if we have many different types in our manifest.
RelayRouter__ + top level RelayRouter
.
In #78 I've changed <script type="module" src="x"/>
insertions to a dynamic import
which is slightly more succinct and has the same effect.
The drawback is that Vite now knows we're loading something but can't analyse it and will complain
2:59:05 PM [vite] warning:
/rescript-relay-router/packages/rescript-relay-router-core/src/RelayRouter__Asse
25 | return ;
26 | } else {
27 | import(asset._0.__$rescriptChunkName__);
| ^
28 | return ;
29 | }
The above dynamic import cannot be analyzed by vite.
See https://github.com/rollup/plugins/tree/master/packages/dynamic-import-vars#limitations for supported dynamic import formats. If this is intended to be left as-is, you can use the /* @vite-ignore */ comment inside the import() call to suppress this warning.
@vite-ignore */ comment inside the import() call to suppress this warning.
Plugin: vite:import-analysis
File: /Users/alexander/Projects/rescript-relay-router/packages/rescript-relay-router-core/src/RelayRouter__AssetPreloader.mjs
I think we have three options to get rid of this warning
Component
(which I think we removed in the past 😅 ) rather than passing a variable to import
. This has the upside that Vite may be able to do smart things with our dynamic imports/* @vite-ignore */
string to our import in this case if we think Vite optimizing our dynamic import would cause issues.React has a good set of lints for things like #26
By creating a test step in this repository that runs ESLint with those lints against our compiled JavaScript code we can make sure that our Rescript code also adheres to those standards.
Similarly in the following code (until line 274) it would probably tell us two things:
rescript-relay-router/router/RelayRouter.res
Line 267 in 1e685ab
use*Effect
will not be called on the server so it's safe to keep in server side code)router
and routeEntry
dependencies.Will make it easier to find what you're looking for. Essentially, as skeletons for route renderers are emitted, emit them in the corresponding folders.
# Everything is flat now
- routes
Root__Dashboard__Organization__Details_route_renderer.res
# Can do it like this
- routes
- Dashboard
- Organization
- Details
Root__Dashboard__Organization__Details_route_renderer.res
There's a few things relying on the current flat structure, but I think it'll be easy changing.
Biggest problem is I'll need a migration script for this for my own codebase that has enough routes that doing that by hand would be majorly painful...
Up for debate what the best pattern is here.
I've noticed that the Routes..Route module won't update in case there are other errors in the Rescript project. This is quite problematic in case the error in the project is a type mismatched caused by code in that module (e.g. the query parameters were incorrectly typed). To fix this the user must first remove the usage of the query parameter to make the project compile and then rebuild the project.
It's currently easy to change query parameters, but it'd be cool to have something that allows updating path parameters easily as well.
An example use case:
Imagine you have a backoffice where you can navigate to different pages to administer organizations. The organization slug is a path parameter in the URL. The page also has a search bar at the top, where you can search for organizations and move to their backoffice.
Now, imagine you want to stay at the current page when searching in that bar, just switch out what org you're looking at. This is pretty painful to do today. But, if we have something like Routes.Backoffice.Dashboard.updatePathParams(~slug=newSlug)
then that would be very easy instead.
cc @Kingdutch , what do you think?
A way to just paste a link to any page in your app, and via that navigate to the route renderers responsible for rendering that route.
Warning: usePreloadedQuery(): Expected preloadedQuery to not be disposed yet. This is because disposing the query marks it for future garbage collection, and as such query results may no longer be present in the Relay store. In the future, this will become a hard error.
at HomeRoot (http://localhost:3000/src/HomeRoot.bs.js:115:24)
at make (http://localhost:3000/src/routes/Root__HomeRoot_route_renderer.bs.js:53:15)
at RelayRouter$RouteComponent (http://localhost:3000/node_modules/.vite/deps/rescript-relay-router_src_RelayRouter_bs_js.js?v=119ac42f:217:16)
at Suspense
at section
at div
at Root (http://localhost:3000/src/Root.bs.js:114:24)
at make (http://localhost:3000/src/routes/Root_route_renderer.bs.js:54:15)
at RelayRouter$RouteComponent (http://localhost:3000/node_modules/.vite/deps/rescript-relay-router_src_RelayRouter_bs_js.js?v=119ac42f:217:16)
at Suspense
at RelayRouter$RouteRenderer (http://localhost:3000/node_modules/.vite/deps/rescript-relay-router_src_RelayRouter_bs_js.js?v=119ac42f:220:29)
at ErrorBoundary (http://localhost:3000/node_modules/.vite/deps/@rescript_react_src_RescriptReactErrorBoundary_bs_js.js?v=119ac42f:12:16)
at Suspense
at RescriptRelay$Context$Provider (http://localhost:3000/node_modules/.vite/deps/rescript-relay_src_RescriptRelay_bs_js.js?v=119ac42f:2144:27)
Reproducible here: https://github.com/cometkim/rescript-relay-issue-tracker
...more things? cc @Kingdutch
https://github.com/cometkim/rescript-vitest
As per Discord discussion :)
Not highly prioritized, but it'd be nice for the CLI to be able to dump a full JSON structure for the routes, that includes everything you'd need if you wanted to do something custom outside of the CLI.
As outlined in #67 the suspense boundary of the router has been removed. This provides the application developers the power (but also the responsibility) on what should be rendered before we start sending data to the client. This is mentioned in the React Docs briefly but desires further explanation.
Multipart currently isn't handled, mostly because it has been really hard to find a setup for fetching multipart responses that also work server side without issues. Once that's up and running it should essentially just be about setting the final
flag correctly in the responses pushed via the stream.
I found at least one example where a non-optional parameter comes before an optional named parameter. This results in making the optional named parameter non-optional or requiring another unit argument.
I would propose to change this to
let isRouteActive = (~exact: bool=false, {pathname}: RelayRouter.Bindings.History.location): bool => {
This enables the following to work without errors:
Route.isRouteActive(location)
Route.isRouteActive(~exact=false, location)
location->Route.isRouteActive
location->Route.isRouteActive(~exact=false)
We can use this issue to find other places where ergonomics can be improved.
The router relies on some requestIdleCallback for some functionality. However, this is not supported by the latest versions of Safari on desktop and mobile.
We should use a fallback in case requestIdleCallback is not available.
A code action that adds a new route to a position, as in inserts the route object in its entirety, and maybe places the cursor at the correct place.
We changed how we did asset preloading which means the preload function is no longer in the router but now in user land. However, we didn't update the README in #55 which means that following the README no longer leads to a working application.
The following errors were encountered while updating to beta24
FAILED: src/entry.client.cmj
We've found a bug for you!
/Users/alexander/Projects/front-end/packages/app/src/entry.client.res:7:33-14:1
5 │
6 │ // Configure our router. Cleanup is unused on the client.
7 │ let (_cleanup, routerContext) = RelayRouter.Router.make(
8 │ ~routes=RouteDeclarations.make(
. │ ...
13 │ ~routerEnvironment=RelayRouter.RouterEnvironment.makeBrowserEnvironme
│ nt(),
14 │ )
15 │
16 │ @val external documentCookie : Js.Nullable.t<string> = "document.cookie
│ "
This call is missing an argument of type
(~preloadAsset: RelayRouter.Types.preloadAssetFn)
rescript: [136/146] src/entry.server.cmj
FAILED: src/entry.server.cmj
We've found a bug for you!
/Users/alexander/Projects/front-end/packages/app/src/entry.server.res:12:34-19:3
10 ┆
11 ┆ // Configure our router.
12 ┆ let (cleanup, routerContext) = RelayRouter.Router.make(
13 ┆ ~routes=RouteDeclarations.make(
. ┆ ...
18 ┆ ~routerEnvironment=RelayRouter.RouterEnvironment.makeServerEnvironmen
┆ t(~initialUrl=url),
19 ┆ )
20 ┆
21 ┆ Promise.make((resolve, reject) => {
This call is missing an argument of type
(~preloadAsset: RelayRouter.Types.preloadAssetFn)
Currently, each route renderer is code split, but the infra that loads each route is always emitted in one single file. This means that while we don't pay bundle costs for route renderers and how much code each route loads, we're still paying costs for every route we add, whether it's used or not.
I'd like to experiment with code splitting on route definition files. If this works out, it means we can replicate the "multiple entry points" pattern (like having a single app that has an admin section baked in for example) with very little bundle size cost. Which would be cool!
As discussed in #23 the history
API is currently private to the router, though the API itself can be very useful for applications who need access to this data (e.g. for navigation or analytics).
This ticket proposes to move the history
bindings to a standalone package in the repository that can be published to NPM so that other projects may use history
without the router. To make consumption by projects using the router easy we can simple re-export the module to avoid projects using the router from having to depend on the history packages themselves.
#Problem
The RelayRouter RouteRenderer component currently renders a suspense boundary. This prevents applications from building up a minimum amount of loaded data before sending a UI to the client.
We rely on the onShellReady
to start sending data to the client. React 18 considers the "shell" to be "anything that's not within a suspense boundary". This provides the UI engineer power to determine how fleshed out the app-shell should be and define a data-minimum.
Because the RouteRenderer renders a suspense boundary, no data loading (leveraging the router) can happen to produce an app shell. This prevents e.g. rendering at least a full header on the server side while allowing the rest of the content to stream in.
Remove the renderFallback
argument to RelayRouter.RouteRenderer
and do not render a Suspense boundary in the router itself. This allows applications to define what data they want loaded before responding to the client.
Users who want to send something to the client without waiting for any data can still achieve this by defining the suspense boundary around the router themselves.
Replacing:
<RelayRouter.RouteRenderer
// Fallback renders whenever suspense bubbles all the way up here, to the router
renderFallback={_ => React.string("Fallback...")}
// This renders all the time, and when there's a pending navigation (pending via React concurrent mode), pending will be `true`
renderPending={pending => pending ? React.string("Pending....") : React.null}
/>
with
<React.Suspense fallback={React.string("Fallback...")}>
<RelayRouter.RouteRenderer
// This renders all the time, and when there's a pending navigation (pending via React concurrent mode), pending will be `true`
renderPending={pending => pending ? React.string("Pending....") : React.null}
/>
</React.Suspense>
ReplaySubject's are used to replay Relay data that was retrieved server side, client side. This is how shipping data from the server to the client is handled. However, we need to determine when a replay subject is cleaned up/removed (so it's not accidentally used again when we want fresh data from the API via the client), and this is slightly messy for various reasons.
This ticket writes down some thoughts what could be part of the public API. This is based on trying to use the router in an application.
RelayRouter__Utils implement hooks and helper functions that should probably all be available on RelayRouter
itself. I think it may make sense to even skip the .Utils
namespace here since they'll be commonly accessed.
I do notice that isRouteActive
and useIsRouteActive
are also on the individual route modules. Are they both supposed to be used or should only the route specific ones be accessed?
RelayRouter__Utils.useLocation returns a RelayRouter__Bindings.History.location entry. This means that the bindings for History
are needed to actually get anything from this value.
One thing I was trying to do was to create a login route that redirected to the current page. However, there's no easy way to assemble a path (including query string and anchor) for the current page from a location object.
/
/todos
Could not reproduce in production mode by running yarn build && yarn preview
.
Warning: usePreloadedQuery(): Expected preloadedQuery to not be disposed yet. This is because disposing the query marks it for future garbage collection, and as such query results may no longer be present in the Relay store. In the future, this will become a hard error.
Layout@http://localhost:9999/src/components/Layout.mjs:101:18
RelayRouter$RouteComponent@http://localhost:9999/router/RelayRouter.mjs:178:3
RelayRouter$RouteRenderer@http://localhost:9999/router/RelayRouter.mjs:182:23
ErrorBoundary@http://localhost:9999/node_modules/.vite/deps/@rescript_react_src_RescriptReactErrorBoundary_mjs.js?v=0d390227:12:16
App
ErrorBoundary@http://localhost:9999/node_modules/.vite/deps/@rescript_react_src_RescriptReactErrorBoundary_mjs.js?v=0d390227:12:16
Suspense
RescriptRelay$Context$Provider@http://localhost:9999/node_modules/.vite/deps/rescript-relay_src_RescriptRelay_mjs.js?v=0d390227:353:21
Main@http://localhost:9999/src/Main.mjs:11:21
Document how to handle routes that require authentication or routes that find they don't have access to a resource after trying to fetch this from a remote resource (e.g. anonymous access is allowed in general but for this resource authentication is needed).
Good idea to build and install the package in the future example project, to ensure everything keeps working between each commit.
Code action that adds a full new route file a position, and opens that file.
No point in having it, can just call the CLI rescript-relay-router
.
We currently have type safe route creators. They're fantastic for generating links in a type safe way. However, they're not great for generating new URLs from existing ones.
Imagine I'm in a backoffice on /organization/123/members?search=Name&order=asc&orderBy=created
. Switching any of the query params here is easy thanks to our Route.useQueryParams
hook which let us spread the old params before setting new params, allowing us to just update the query params we want while retaining the rest. This hook also performs an actual update, it doesn't give me back a URL I can stick in a link.
Imagine I want to switch out what organization I'm on. I'll need to run Route.makeLink
again and re-insert all query params by hand to the value they currently are. Depending on where those query params are controlled, it might be easy or hard to even access them where I'm creating my link.
Proposed API:
let {makeLink} = Route.useDynamicLink()
<RelayRouter.Link to_={makeLink(~params=currentParams => {...currentParams, organizationId: Some(newOrgId)}, ())
The difference to the current method exposed by Route.useQueryParams
is that this would also allow setting any path params, and generate a full new link from that. If we leave it at just generating the actual URL, one could delegate all of the "tough" decisions around "should I update the URL or not?" to the user by letting them choose - stick it in a link, use the programmatic push/replace
API (with potential shallow
support in the future), etc.
This could be generated in addition to the current makeLink
, which still would be the most used API.
The generated route declarations currently pull in makePrepareProps
and makeRouteKey
from the generated route files. But, we know how to generate both of these, so we could inline them into the generated code directly, instead of risking bundle bloat because we're pulling in just a small part of a larger generated file.
Since we use jsonc
under the hood, we're allowed to have comments in our route definition files. See if we can pick up these comments when they're on the lines above a route definition, so we can reprint them to appear in ReScript's hover.
We've chatted about it for a bit but since Discord is down I figured I'd create a GitHub issue instead 🙃
I'd like to move the repository closer to a monorepo structure so we no longer need the buildPackage.sh
script. This would also make it easier to figure out what goes in "Application" and what goes in "Router" (and supporting packages).
I'm thinking of something like this:
.vscode
examples/
cloudflare-workers/
express/
packages/
router/
test/
router-vite-plugin/
test/
router-cli/
test/
The existing src/
would be moved into express/
and I'd like to add a cloudflare worker example right after using miniflare to ensure that the router itself does not depend on a specific server.
The idea is that when changes to any of the packages are made the example application tests are also run to serve as integration tests.
The proposed structure and package names (I won't change the name in the package.json) assumes a few things:
router-
prefix so that we have the option to merge this into a single rescript-relay
monorepo. With the GraphQL client and router. However, the WIP.md mentions that the router should be relay independent. I think this also means that some of the CLI/VSCode plugin/Streaming code should be moved from Relay to the router (not sure which code though)?rescript-relay-router rescript-relay-router-vite-plugin
). I'm not sure about the CLI. (we could of course still depend on any of the sub-packages from router
itself, but at least for adding new plugins and not installing all of them that'd require a major version).The repository contains some files for a more full-fledge application that I don't think are needed for development of the router or for the examples. Do those have a purpose or are they historic artifacts that we could remove?
Specifically:
Find some sort of workflow for this. Can the editor tooling handle this somehow? Maybe a rename provider that does all of this?
This needs investigation, but I suspect our mechanism for preventing prepare being called multiple times might block Relay store invalidations. I have a few ideas on how to solve this.
A declarative, efficient, and flexible JavaScript library for building user interfaces.
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
An Open Source Machine Learning Framework for Everyone
The Web framework for perfectionists with deadlines.
A PHP framework for web artisans
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
Some thing interesting about web. New door for the world.
A server is a program made to process requests and deliver data to clients.
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
Some thing interesting about visualization, use data art
Some thing interesting about game, make everyone happy.
We are working to build community through open source technology. NB: members must have two-factor auth.
Open source projects and samples from Microsoft.
Google ❤️ Open Source for everyone.
Alibaba Open Source for everyone
Data-Driven Documents codes.
China tencent open source team.