Skip to content

Conversation

@mcansh
Copy link
Owner

@mcansh mcansh commented Apr 1, 2025

closes #514

  • chore: use latest versions of react-router packages
  • feat: update types for server file middleware

mcansh added 2 commits April 1, 2025 19:01
Signed-off-by: Logan McAnsh <logan@mcan.sh>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
@changeset-bot
Copy link

changeset-bot bot commented Apr 1, 2025

⚠️ No Changeset found

Latest commit: 9046e9d

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

@mcansh mcansh marked this pull request as ready for review April 1, 2025 23:31
@mcansh mcansh force-pushed the logan/fix-middleware branch from 74bc896 to 79205e7 Compare April 1, 2025 23:32
mcansh added 2 commits April 1, 2025 20:01
…s in the examples

Signed-off-by: Logan McAnsh <logan@mcan.sh>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
@mcansh mcansh force-pushed the logan/fix-middleware branch from 6628524 to 1c069f9 Compare April 2, 2025 00:17
@mcansh mcansh requested a review from Copilot April 2, 2025 00:17
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR updates and refactors middleware integration and dependency handling for the remix-fastify package and its examples. Key changes include removing the middleware bundle entry and associated files, updating react-router types and context handling, and revising dependency updates in the pnpmfile.

  • Removed middleware entry and related files from the remix-fastify package.
  • Updated type definitions and context management for react-router integration.
  • Enhanced dependency resolution in .pnpmfile.cjs for examples using remix-related packages.

Reviewed Changes

Copilot reviewed 16 out of 23 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
packages/remix-fastify/tsup.config.ts Removed middleware entry from build and updated re-export paths.
packages/remix-fastify/src/shared.ts Removed an unused import from react-router.
packages/remix-fastify/src/react-router.ts Updated type definitions for the load context with conditional types.
packages/remix-fastify/src/middleware.ts Removed middleware file as it is no longer used.
packages/remix-fastify/middleware.{js, d.cts, cjs} Removed legacy middleware export files.
examples/react-router/vite.config.ts Simplified plugin configuration for react-router usage.
examples/react-router/server.ts Updated plugin registration with a getLoadContext option.
examples/react-router/react-router.config.ts Added configuration for enabling unstable middleware.
examples/react-router/context.ts Added a new context creation file with default adapter context.
examples/react-router/app/root.tsx Updated context usage and middleware configuration in the app root.
.pnpmfile.cjs Revised dependency resolution to update remix-related dependencies.
Files not reviewed (7)
  • examples/basic/package.json: Language not supported
  • examples/basic/tsconfig.json: Language not supported
  • examples/playground/package.json: Language not supported
  • examples/react-router/package.json: Language not supported
  • examples/react-router/tsconfig.json: Language not supported
  • examples/vite/package.json: Language not supported
  • package.json: Language not supported
Comments suppressed due to low confidence (1)

examples/react-router/context.ts:8

  • [nitpick] The default value 'lol default value' might be too informal for production. Consider updating it to a more descriptive and professional string.
session: "lol default value",

mcansh and others added 7 commits April 1, 2025 20:18
Signed-off-by: Logan McAnsh <logan@mcan.sh>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
@pkg-pr-new
Copy link

pkg-pr-new bot commented Apr 2, 2025

More templates

npm i https://pkg.pr.new/@mcansh/remix-fastify@515

commit: 45e753a

@mcansh mcansh changed the title logan/fix middleware feat: server middleware Apr 2, 2025
@jpinn97
Copy link

jpinn97 commented Apr 2, 2025

Hi, thanks for getting onto this quickly. Had no issue with the react-router side of it, but I've observed no effect from getLoadContext under the current implementation and double checking against the 7.3 patch notes too. I have been able to pass other things through such as Symbols, and retrieve them, so this works.


// server.ts
export const CONTEXT_KEY = Symbol.for('adapter-context-key');

function getAdapterContext(request: FastifyRequest) {
  return {
    session: "test"
  };
}

// @ts-expect-error
await app.register(reactRouterFastify, {
  getLoadContext(request): unstable_InitialContext {
    let map = new Map();
    map.set(adapterContext, getAdapterContext(request));
    
    map.set(CONTEXT_KEY, getAdapterContext(request));
    
    console.log("Setting context:", map);
    return map;
  },
});

// root.tsx
export function loader({ context }: Route.LoaderArgs) {
  // Get the context using the Symbol approach
  const symbolContextValue = context.get(CONTEXT_KEY);
  
  console.log('Symbol context get:', { symbolContextValue });
  
  return null;
}

export const unstable_middleware: unstable_MiddlewareFunction[] = [
  async ({ context }) => {
    // Get the context using the Symbol approach in middleware too
    const symbolContextValue = context.get(CONTEXT_KEY);
    
    console.log('Middleware symbol context:', { symbolContextValue });
  },
];

Obviously this isn't the context, or the correct implemtation, but essentially to prove it passes through.
I cannot set the map however, context.get(adapterContext continues to return the map with default value, so not exactly sure what happening, either not updating the context or the context is being overwrote in rr?

@mcansh
Copy link
Owner Author

mcansh commented Apr 2, 2025

Hi, thanks for getting onto this quickly. Had no issue with the react-router side of it, but I've observed no effect from getLoadContext under the current implementation and double checking against the 7.3 patch notes too. I have been able to pass other things through such as Symbols, and retrieve them, so this works.




// server.ts

export const CONTEXT_KEY = Symbol.for('adapter-context-key');



function getAdapterContext(request: FastifyRequest) {

  return {

    session: "test"

  };

}



// @ts-expect-error

await app.register(reactRouterFastify, {

  getLoadContext(request): unstable_InitialContext {

    let map = new Map();

    map.set(adapterContext, getAdapterContext(request));

    

    map.set(CONTEXT_KEY, getAdapterContext(request));

    

    console.log("Setting context:", map);

    return map;

  },

});



// root.tsx

export function loader({ context }: Route.LoaderArgs) {

  // Get the context using the Symbol approach

  const symbolContextValue = context.get(CONTEXT_KEY);

  

  console.log('Symbol context get:', { symbolContextValue });

  

  return null;

}



export const unstable_middleware: unstable_MiddlewareFunction[] = [

  async ({ context }) => {

    // Get the context using the Symbol approach in middleware too

    const symbolContextValue = context.get(CONTEXT_KEY);

    

    console.log('Middleware symbol context:', { symbolContextValue });

  },

];

Obviously this isn't the context, or the correct implemtation, but essentially to prove it passes through.

I cannot set the map however, context.get(adapterContext continues to return the map with default value, so not exactly sure what happening, either not updating the context or the context is being overwrote in rr?

do you have the required lines for typescript in your react router config? can you provide a minimal reproduction repo just to be safe?

@jpinn97
Copy link

jpinn97 commented Apr 2, 2025

I've tried this PR as is. Cloned the PR bot template, so its as is, are you saying it works for you?

@mcansh
Copy link
Owner Author

mcansh commented Apr 2, 2025

I've tried this PR as is. Cloned the PR bot template, so its as is, are you saying it works for you?

yep, aside from the one // @ts-expect-error i have in server.ts when registering the plugin

@jpinn97
Copy link

jpinn97 commented Apr 2, 2025

image

Well I don't know then, cloned your monorepo fresh. My expectation is this would print out abc for the first loader get context, not the default value?

@mcansh
Copy link
Owner Author

mcansh commented Apr 2, 2025

i'll look into some more this evening. here im required to add an initial value, but in an express app im not

…pmfile

Signed-off-by: Logan McAnsh <logan@mcan.sh>
@mcansh mcansh force-pushed the logan/fix-middleware branch from 45e753a to b80fe2c Compare April 23, 2025 23:20
mcansh added 3 commits April 23, 2025 19:21
Signed-off-by: Logan McAnsh <logan@mcan.sh>
Signed-off-by: Logan McAnsh <logan@mcan.sh>
Deletes the PostCSS configuration file as it is no longer needed.

Cleans up the project by removing unnecessary files.

Signed-off-by: Logan McAnsh <logan@mcan.sh>
…pmfile

Signed-off-by: Logan McAnsh <logan@mcan.sh>
(cherry picked from commit b80fe2c)
Signed-off-by: Logan McAnsh <logan@mcan.sh>
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

Successfully merging this pull request may close these issues.

React Router 7 GetLoadContext not available when using unstable_middleware

3 participants