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

inject middleware instead of using router #75

Merged

Conversation

Boegie19
Copy link
Member

@Boegie19 Boegie19 commented Oct 12, 2023

after talking to derrick this is my PR for chancing from router usage to injection usage.

Possible Breaking Change: ONLY strapi.api routes can be seen.

fixes: #65

Copy link

@derrickmehaffy derrickmehaffy left a comment

Choose a reason for hiding this comment

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

Thinking about this it's gonna be basically impossible to cache plugin routes 🤔 because the plugins won't even be loaded yet let alone register their routes. Especially here where my redis plugin hasn't even ran yet unless I modify it I guess to run on register too? Not sure on the timing though as I basically need to get my plugin registered first.

const { resolveUserStrategy } = require('./utils/config/resolveUserStrategy');
const { injectMiddlewares } = require('./utils/middlewares/injectMiddlewares');

const createProvider = async (providerConfig, { strapi }) => {

Choose a reason for hiding this comment

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

This needs to be moved back to bootstrap currently it will try to run this before the provider exists:

image

We can still register the middlewares but we can't create the provider until we get to the bootstrap.

@derrickmehaffy
Copy link

Btw I fixed a bug you had with the contentType name as that doesn't exist.

@derrickmehaffy
Copy link

Tested moving my code in strapi-plugin-redis over to register instead of bootstrap and it works, if we can move the provider registration to bootstrap that would be best instead of me doing this in register as it might fuck up other plugins.

@derrickmehaffy
Copy link

After doing some basic testing it works really well and custom additional route middlewares work perfectly 👍

@Boegie19
Copy link
Member Author

Will make the provider change this evening

@derrickmehaffy
Copy link

Can't promise I'll have time to test this before I go on vacation but I'll try

@pragmago
Copy link

@derrickmehaffy @Boegie19 is there anything we can do to push this along?

@derrickmehaffy
Copy link

@derrickmehaffy @Boegie19 is there anything we can do to push this along?

We will most likely wait for Strapi v5 to be released as it's gonna require an entire new rewrite most likely

@derrickmehaffy derrickmehaffy changed the base branch from main to dev/strapi5 November 4, 2024 20:02
Copy link

@derrickmehaffy derrickmehaffy left a comment

Choose a reason for hiding this comment

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

LGTM

@derrickmehaffy derrickmehaffy merged commit b6bf53c into dev/strapi5 Nov 4, 2024
@derrickmehaffy derrickmehaffy deleted the inject-middleware-instead-of-using-router branch November 4, 2024 20:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement RFC-01 for changing the way cache middlewares are injected into the application
3 participants