Skip to content
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

Add support for matching internal middlewares #79

Draft
wants to merge 15 commits into
base: master
Choose a base branch
from

Conversation

marcospassos
Copy link
Member

@marcospassos marcospassos commented Oct 13, 2024

Summary

  • Refactors middleware handling to simplify integration with existing custom middleware matches.
  • Introduces a new option in withCroct to allow easier handling of both custom and Croct middlewares.
  • Automatically forwards headers set by Croct’s middleware, avoiding common issues.
  • Marks internal functions as @internal to prevent unintended usage.

Improvements

When you're working on a project that already has middleware and want to add Croct's middleware, things can get tricky—especially if there's a matcher in place to determine which routes the middleware should handle.

For example, let's say you have the following middleware setup:

export const config = {
    matcher: '/api/*',
};

export const middleware = () => {
    console.log('Middleware');

    return NextResponse.next();
}

Since Croct's middleware needs to run on all pages (not all paths), you have to refactor this to handle the matching logic inside the middleware itself. But refactoring like this can get complicated, especially if your matcher is complex.

For the example above, you would need to refactor the middleware like this:

export {config} from '@croct/plug-next/middleware';
import {withCroct} from '@croct/plug-next/middleware';

export const middleware = withCroct(request => {
    if (request.nextUrl.pathname.startsWith('/api')) {
        console.log('Middleware');
    }

    return NextResponse.next({
        request: {
            headers: new Headers(request.headers),
        },
    });
});

While this works, the more complex the matcher, the more challenging this refactoring becomes.

New Solution

This PR introduces a new option to the withCroct function that lets you pass in the existing matcher directly. This way, you can make sure your existing middleware only runs when its matcher conditions are met, while Croct's middleware still runs across all pages without requiring complicated refactoring.

Here's how you can simplify the previous example with this update:

import {config: {matcher: croctMatcher}, withCroct} from '@croct/plug-next/middleware';

export const config = {
    matcher: ['/api/*', ...croctMatcher],
};

export const middleware = withCroct({
    next: request => {
        console.log('Middleware');

        return NextResponse.next({
            request: {
                headers: new Headers(request.headers),
            },
        });
    },
    matcher: config.matcher,
});

Header Forwarding Fix

Another key improvement: you no longer need to manually forward headers to the next middleware. Before this, if you forgot to forward headers, you'd lose the ones set by Croct's middleware, which was a common issue when refactoring existing middleware.

This PR solves that by automatically wrapping the NextResponse.next function to ensure that all headers are forwarded correctly, preventing these issues.

With these changes, the example can be further simplified:

import {config: {matcher: croctMatcher}, withCroct} from '@croct/plug-next/middleware';

export const config = {
    matcher: ['/api/*', ...croctMatcher],
};

export const middleware = withCroct({
    next: () => console.log('Middleware'),
    matcher: config.matcher,
});

Internal Function Marking

Lastly, to clean up the library, all internal functions are now marked as @internal, making it clear that they shouldn’t be used outside of the library itself.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

@marcospassos marcospassos added bc-break This change is not backward-compatible feature New feature labels Oct 13, 2024
Copy link

pkg-pr-new bot commented Oct 13, 2024

Open in Stackblitz

npm i https://pkg.pr.new/@croct/plug-next@79

commit: 2dd7cb2

@marcospassos marcospassos force-pushed the middleware-improvements branch from 07deb71 to 1d77e06 Compare October 23, 2024 19:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bc-break This change is not backward-compatible feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant