Add integration tests for request structures #4829

brophdawg11 · 2022-12-12T16:39:51Z

Adds additional tests to the bug report in #4826. I went back and made sure all these tests pass on 1.7.6 and then with the sibling PR in react-router they all pass on 1.8.x Remix now as well.

Note: CI will fail until we release RR and update server-runtime's package.json in this branch

React Router PR: remix-run/react-router#9721

Can test via:

Check out this branch in remix
Check out the corresponding branch in react-router
In react-router,
- LOCAL_BUILD_DIRECTORY=../remix yarn build
- Assumes your remix and react-router repo clones are sibling directories
In remix:
- yarn test:integration request-test --project chromium

Also, since this PR will update the @remix-run/router dependency I also added an integration tests for #4791 which ids fixed by remix-run/react-router#9702

brophdawg11 · 2022-12-12T16:44:12Z

integration/request-test.ts

+  expect(loaderData.method).toEqual("POST");
+  expect(loaderData.url).toMatch(/^http:\/\/localhost:\d+\/$/);
+  expect(loaderData.headers.cookie).toEqual("cookie=nomnom");
+  expect(loaderData.text).toEqual("");


During MPA submissions, the loader request proxies the action method (POST) but strips the body. This makes sense since there's only a single submission happening.

brophdawg11 · 2022-12-12T16:46:26Z

integration/request-test.ts

+  loaderData = JSON.parse(await page.locator("#loader-data").innerHTML());
+  expect(loaderData.method).toEqual("GET");
+  expect(loaderData.url).toMatch(/^http:\/\/localhost:\d+\/$/);
+  expect(loaderData.headers.cookie).toEqual("cookie=nomnom");
+  expect(loaderData.text).toEqual("");


During SPA submissions, the loader is called through separate revalidation GET requests, so the assertions here differ on the method.

I think this all makes sense given how Remix is executing loaders differently in MPA/SPA calls, even if it introduces a slight different between them from a DX standpoint. Users shouldn't be branching on request.method in loaders anyway?

This logic is implemented in the router here: remix-run/react-router#9721

changeset-bot · 2022-12-12T17:02:30Z

🦋 Changeset detected

Latest commit: 7a3c205

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 17 packages

Name	Type
@remix-run/server-runtime	Patch
@remix-run/cloudflare	Patch
@remix-run/deno	Patch
@remix-run/dev	Patch
@remix-run/node	Patch
@remix-run/react	Patch
@remix-run/testing	Patch
@remix-run/cloudflare-pages	Patch
@remix-run/cloudflare-workers	Patch
create-remix	Patch
@remix-run/architect	Patch
@remix-run/express	Patch
@remix-run/netlify	Patch
@remix-run/vercel	Patch
@remix-run/serve	Patch
remix	Patch
@remix-run/eslint-config	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

brophdawg11 · 2022-12-13T20:54:00Z

integration/catch-boundary-test.ts

+
+  test("prefers parent catch when child loader also bubbles, document request", async () => {
+    let res = await fixture.requestDocument(`${HAS_BOUNDARY_LOADER}/child`);
+    expect(res.status).toBe(401);


parent throws 401, child throws 404

brophdawg11 · 2022-12-13T20:59:55Z

integration/request-test.ts

+  expect(actionData.text).toEqual("type=ssr");
+
+  loaderData = JSON.parse(await page.locator("#loader-data").innerHTML());
+  expect(loaderData.method).toEqual("GET");


Loaders receive method:GET on SSR "revalidations" now. This fixes a bug in Remix 1.7.6 where the loader request was POST but without a body, and thus made for a different DX if you did a SSR/CSR submissions since request.method would be different in the loader.

* fix(remix-dev): convert `config.appDirectory` to relative unix path (#4709) * fix(remix-dev): convert appDirectory to unix style for fast-glob Signed-off-by: Logan McAnsh <logan@mcan.sh> * chore: relative path Signed-off-by: Logan McAnsh <logan@mcan.sh> * chore: add test Signed-off-by: Logan McAnsh <logan@mcan.sh> * fix test Signed-off-by: Logan McAnsh <logan@mcan.sh> * fix: typo Signed-off-by: Logan McAnsh <logan@mcan.sh> * chore: update test Signed-off-by: Logan McAnsh <logan@mcan.sh> Signed-off-by: Logan McAnsh <logan@mcan.sh> * chore: add changeset for #4709 (#4718) * ci: add typechecking for deno (#4715) * ci: add typechecking for deno * ci: install deno for integration tests * chore: format * chore: format * fix: Firefox LiveReload (#4725) Firefox infinitely reloads the page as long as `<LiveReload>` is rendering. Closes #4692 * fix(remix-dev): allow defining multiple routes for the same route module file (#3970) * Allow multiple routes for same route module * Update packages/remix-dev/config/routes.ts Co-authored-by: Andrew Leedham <AndrewLeedham@outlook.com> * Update routes.ts - Better name for automated ID variable; - Small adjust in `id` option comment; * - Removing redundant IF * Update routes.ts Revert complex custom ID in routes * Non unique custom routes ID error and test * Update assets.ts Trying to solve a conflict * Revert "Update assets.ts" This reverts commit 2064c57. * Error on collisions with non-custom routeIds * Create big-spoons-grab.md Co-authored-by: Andrew Leedham <AndrewLeedham@outlook.com> Co-authored-by: Matt Brophy <matt@brophy.org> * feat: Allow pass-through script props in `ScrollRestoration` (#2879) * ci(nightly): add deno for typechecking deno files (#4738) * ci: fix race condition writing globals.d.ts shim (#4717) Co-authored-by: Chance Strickland <hi@chance.dev> * chore: bump @playwright/test to latest (#4749) Signed-off-by: Logan McAnsh <logan@mcan.sh> Signed-off-by: Logan McAnsh <logan@mcan.sh> * Fix 4199: TypedResponse allows incompatible types (#4734) * Fixes #4199: Do not allow assignment of incompatible TypedResponses * Add myself to contributors.yml * Create light-sheep-give.md * slight changeset tweak * additional changeset tweaks Co-authored-by: Pedro Cattori <pcattori@gmail.com> * chore: format * test: add transition integration tests (#4739) test: useTransition to wait for states This approach could probably be applied across other flakey tests and could also be documented as a good approach of if there is user-feedback for a specific action when running integration tests. * feat: testing helpers (#4539) Co-authored-by: James Restall <james.restall@gmail.com> Signed-off-by: Logan McAnsh <logan@mcan.sh> * chore: format * chore(remix-testing): update dependencies (#4756) * fix(remix-testing): fix deps (#4757) * Fix deps for new remix-testing package * fix lint * Remove ENABLE_REMIX_ROUTER flags (#4732) * chore: add `@remix-run/testing` to changesets (#4781) * chore: add `@remix-run/testing` to changesets Signed-off-by: Logan McAnsh <logan@mcan.sh> * chore: update `ADDING_A_PACKAGE.md` Signed-off-by: Logan McAnsh <logan@mcan.sh> Signed-off-by: Logan McAnsh <logan@mcan.sh> * refactor(remix-react): upgrade Remix to `react-router-dom@6.4` (non-data-router) and drop `history` (#4731) * Bump remix to react-router-dom@6.4.4 (#4668) Co-authored-by: Mehdi Achour <machour@gmail.com> * Bump remix to RR 6.4.4 and drop history dependency (#4702) Co-authored-by: Mehdi Achour <machour@gmail.com> * ci(nightly): move git operations after build (#4797) * ci(nightly): move git operations after build when adding deno typechecking (#4715), nightly now refers to the version we just calculated which hasnt been published when trying to build and typecheck Signed-off-by: Logan McAnsh <logan@mcan.sh> * ci: update tmp branch name Signed-off-by: Logan McAnsh <logan@mcan.sh> Signed-off-by: Logan McAnsh <logan@mcan.sh> * perf(remix-dev): Optimize `parentRouteId` lookup in `defineConventionalRoutes` (#4538) * Use object for parentRouteId lookup * Sign CLA * Move parentRouteId logic to a separate function * Update test fixture path * chore: format * chore(dev): add changeset for PR #4538 (#4800) * chore: format * refactor(remix-react): use `react-router-dom` import instead of `react-router` (#3325) * chore: manually bump remix-testing version due to not being on main just yet Signed-off-by: Logan McAnsh <logan@mcan.sh> * fixup! chore: manually bump remix-testing version due to not being on main just yet * fixup! chore: manually bump remix-testing version due to not being on main just yet Signed-off-by: Logan McAnsh <logan@mcan.sh> * chore: unify error usage (#4696) * chore: format * Add fetcher state/type tests (#4803) * chore(deps): bump esbuild to latest (#4754) * chore: add invariant instead of using `!` Signed-off-by: Logan McAnsh <logan@mcan.sh> * chore(deps): bump esbuild to latest Signed-off-by: Logan McAnsh <logan@mcan.sh> * Create fresh-shrimps-join.md Signed-off-by: Logan McAnsh <logan@mcan.sh> Co-authored-by: Pedro Cattori <pcattori@gmail.com> * test(integration): close server synchronously (#4785) * chore: normalize afterAll Signed-off-by: Logan McAnsh <logan@mcan.sh> * test: close server synchronously Signed-off-by: Logan McAnsh <logan@mcan.sh> * chore: appFixture.close isnt async anymore Signed-off-by: Logan McAnsh <logan@mcan.sh> * Update integration/helpers/create-fixture.ts Co-authored-by: Pedro Cattori <pcattori@gmail.com> Signed-off-by: Logan McAnsh <logan@mcan.sh> Co-authored-by: Pedro Cattori <pcattori@gmail.com> * feat: remix optional segments (#4706) * feat: transform optional routes from remix to react router * Add to contributors * Add changeset * fix(optional-segments): fix escaping of parenthesis * small function fix * Update packages/remix-dev/__tests__/routesConvention-test.ts Co-authored-by: Pedro Cattori <pcattori@gmail.com> Co-authored-by: Pedro Cattori <pcattori@gmail.com> * chore: format * chore: edit the optional segments changeset (#4815) to make it clear that Remix won't support optional segments until integrated with React Router 6.5 * chore: format * docs: rearrange docs, give everything its own page (#4821) * chore: format * fix: wrong parentheses in the optional segments changeset (#4825) * fix: wrong parentheses in the optional segments changeset * add: akamfoad to the contributers * ci(nightly): add workflow_call for testing purposes Signed-off-by: Logan McAnsh <logan@mcan.sh> * Revert "ci(nightly): add workflow_call for testing purposes" This reverts commit f3fa77e. * chore(scripts): Use relaxed peer dependencies to avoid triggering major version bumps (#4736) - Add script to bump peer deps with changesets version Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> * Add integration tests for request structures (#4829) * fix(remix-dev): resolve asset entry full path to support mono-repo import of styles (#4855) * chore: have eslint report unused eslint comments (#4863) * chore: have eslint report unused eslint comments Signed-off-by: Logan McAnsh <logan@mcan.sh> * chore: remove additional comment Signed-off-by: Logan McAnsh <logan@mcan.sh> * perf(remix-architect,remix-netlify): improve performance of `isBinaryType` (#4761) Co-authored-by: Logan McAnsh <logan@mcan.sh> Co-authored-by: Pannatier Guillaume <Guillaume.Pannatier@hopitalvs.ch> * chore: format * chore(remix-testing): remove internal installGlobals (#4755) * chore: remove internal installGlobals Signed-off-by: Logan McAnsh <logan@mcan.sh> * chore: add README Signed-off-by: Logan McAnsh <logan@mcan.sh> * Create quick-cats-fix.md * Update .changeset/quick-cats-fix.md Co-authored-by: Michaël De Boey <info@michaeldeboey.be> * chore(deps): remove jsdom and happydom from devDependencies Signed-off-by: Logan McAnsh <logan@mcan.sh> Signed-off-by: Logan McAnsh <logan@mcan.sh> Co-authored-by: Michaël De Boey <info@michaeldeboey.be> * fix(dev): build js modules for ts->js conversion The TS->JS migration was removed from the CLI codemod options, but still used for TS->JS conversion when creating a new Remix project from the CLI. The TS modules responsible for the TS->JS conversion were incorrectly removed from the Rollup build, resulting in the corresponding built JS modules being absent. That caused the CLI to error when trying to perform TS->JS conversion. This changes reintroduces the wiring to build the modules responsible for the TS->JS conversion. Fixes #4854 Signed-off-by: Logan McAnsh <logan@mcan.sh> Co-authored-by: Logan McAnsh <logan@mcan.sh> Co-authored-by: Matt Kane <m@mk.gg> Co-authored-by: Remix Run Bot <hello@remix.run> Co-authored-by: Chance Strickland <hi@chance.dev> Co-authored-by: Ryan Florence <rpflorence@gmail.com> Co-authored-by: Lucas Ferreira <panchorf@gmail.com> Co-authored-by: Andrew Leedham <AndrewLeedham@outlook.com> Co-authored-by: Matt Brophy <matt@brophy.org> Co-authored-by: dabdine <1955040+dabdine@users.noreply.github.com> Co-authored-by: Jacob Ebey <jacob.ebey@live.com> Co-authored-by: James Restall <james.restall@gmail.com> Co-authored-by: Michaël De Boey <info@michaeldeboey.be> Co-authored-by: Mehdi Achour <machour@gmail.com> Co-authored-by: Dylan Markow <dylan@dylanmarkow.com> Co-authored-by: Daniel Rios <ieldanr@gmail.com> Co-authored-by: Akam Foad <41629832+akamfoad@users.noreply.github.com> Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com> Co-authored-by: Guillaume Pannatier <guillaume.pannatier@gmail.com> Co-authored-by: Pannatier Guillaume <Guillaume.Pannatier@hopitalvs.ch>

Add integration tests for request structures

ebd39db

This was referenced Dec 12, 2022

Fix loader request on document POST requests remix-run/react-router#9721

Merged

bug: v1.8 loaders have empty request.headers when called after action #4826

Closed

brophdawg11 commented Dec 12, 2022

View reviewed changes

brophdawg11 added feat:routing feat:loader package:server-runtime labels Dec 12, 2022

Add changeset

cdacee5

remix-cla-bot bot added the CLA Signed label Dec 12, 2022

brophdawg11 added 3 commits December 12, 2022 13:01

Add tests for duplicate loader boundaries

4373bb9

Bump router versions

1e1b7d8

Update test to reflect GET loader request

d8c78ba

brophdawg11 commented Dec 13, 2022

View reviewed changes

brophdawg11 added 2 commits December 13, 2022 16:01

pin versions

a7e9b36

update changesets

7a3c205

brophdawg11 merged commit 87a0d91 into dev Dec 13, 2022

brophdawg11 deleted the brophdawg11/ssr-submit-loader-request branch December 13, 2022 21:15

github-actions bot added the awaiting release This issue has been fixed and will be released soon label Dec 13, 2022

This was referenced Dec 13, 2022

chore: Update version for release (pre) #4858

Closed

chore: Update version for release (pre) #4869

Closed

chore: Update version for release (pre) #4871

Merged

MichaelDeBoey added the renderer:react label Dec 15, 2022

remix-run-bot mentioned this pull request Dec 16, 2022

chore: Update version for release #4893

Merged

mcansh mentioned this pull request Jan 10, 2023

chore: Update version for release (pre) #5050

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add integration tests for request structures #4829

Add integration tests for request structures #4829

brophdawg11 commented Dec 12, 2022 •

edited

brophdawg11 Dec 12, 2022

brophdawg11 Dec 12, 2022

brophdawg11 Dec 13, 2022

changeset-bot bot commented Dec 12, 2022 •

edited

brophdawg11 Dec 13, 2022

brophdawg11 Dec 13, 2022

Add integration tests for request structures #4829

Add integration tests for request structures #4829

Conversation

brophdawg11 commented Dec 12, 2022 • edited

brophdawg11 Dec 12, 2022

Choose a reason for hiding this comment

brophdawg11 Dec 12, 2022

Choose a reason for hiding this comment

brophdawg11 Dec 13, 2022

Choose a reason for hiding this comment

changeset-bot bot commented Dec 12, 2022 • edited

🦋 Changeset detected

brophdawg11 Dec 13, 2022

Choose a reason for hiding this comment

brophdawg11 Dec 13, 2022

Choose a reason for hiding this comment

brophdawg11 commented Dec 12, 2022 •

edited

changeset-bot bot commented Dec 12, 2022 •

edited