-
Notifications
You must be signed in to change notification settings - Fork 294
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
Incorporate the v3_routeConfig
future flag for Remix.
#2722
base: main
Are you sure you want to change the base?
Conversation
Oxygen deployed a preview of your
Learn more about Hydrogen's GitHub integration. |
|
||
2. In the Vite config (`vite.config.ts` usually) the `remix` plugin needs to have it's configuration slightly altered. | ||
|
||
From this: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would wrap this instruction on a diff -/+
block instead
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great Sean!
Tophat found no issues
import {PageLayout} from '~/components/PageLayout'; | ||
import { RootLoader } from './root'; | ||
|
||
export default function Layout() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think now that we have a separate file for this layout, I would prefer if we consolidated /components/PageLayout.tsx
here to avoid an unnecessary level of abstraction
@@ -16,6 +16,9 @@ | |||
"dependencies": { | |||
"@remix-run/react": "^2.15.3", | |||
"@remix-run/server-runtime": "^2.15.3", | |||
"@remix-run/fs-routes": "^2.15.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A reminder that we will need to add these new dependencies to our changelog.json entry when the time comes to release this
WHY are these changes introduced?
These changes are to support an upgrade to the next version of Remix, by using the future flag and supporting libraries to get the code ready for that upgrade in place before the upgrade itself happens.
WHAT is this pull request doing?
v3_routeConfig
flag set totrue
.hydrogen.v3preset
function which excludes theroutes
attribute that thehydrogen.preset
function includes because that is incompatible with the flag being turned on.hydrogenRoutes
function which transforms the route specified inroutes.ts
to include the virtual routes.getVirtualRoutes
to work without using any node libraries so that it can run in MiniOxygen.LANG
environment variable in thecli
package so that the date formatting uses the US formatting.build.test.ts
now closes resources so that the test doesn't fail due to the hanging file watchers.build.ts
sets theserver.watch
configuration option as that may be necessary to disable the file watching.hydrogen/dev.ts
shuts down the watcher on theviteServer
when finishing up.setup.test.ts
makes sure to include theroutes.ts
file.classic-compiler/dev.ts
makes sure to filter out the virtual routes whendisableVirtualRoutes
is true.common.ts
now makes sure to includeroutes.ts
andlayout.tsx
.local.test.ts
slightly tweaked to make it easier to find out what files are missing and also now correctly takes account of the content that was shifted fromroot.tsx
tolayout.tsx
.remix-config.ts
constructs a valueroutesViteNodeContext
which is necessary when resolving the config ifv3_routeConfig
is enabled.route-validator.ts
is now null safe when assigningcurrentRoute.path
.replacers.ts
looks forlayout
instead ofroot
.get-virtual-routes.test.ts
now checks for thejsx
files instead of thetsx
files of the virtual routes.@remix-run/dev
to not watch inremixVitePlugin
when in a test because the file watchers cannot be reached to switch them off which causes tests relying on that to fail.vite
to handle thewatch
properties of the config to set it if the value isnull
as that has meaning in that particular case.HOW to test your changes?
routes.ts
with thehydrogenRoutes
function, to add the supporting routes.hydrogen.preset()
tohydrogen.v3preset()
.Post-merge steps
Checklist