wpengine / faustjs

Faust.js™ - The Headless WordPress Framework
https://faustjs.org
Other
1.41k stars 126 forks source link

[Feature] Add hook to alter admin url #1882

Open jan-clockworkwp opened 4 months ago

jan-clockworkwp commented 4 months ago

Currently in @faustwp/core: 3.0.1 package, there is a way to hook into wpUrl method. But there is no way to hook into the adminUrl, and as far as there is a way to override SiteName link in the custom toolbar with snippet like this:

hooks.addFilter(
      'toolbarNodes',
      'faust',
      (toolbarNodes: FaustToolbarNodes, context: FaustToolbarContext) => {
        const adminUrl = getAdminUrl().replace('/wp-admin', '/wp/wp-admin');
        const customToolbarNodes: FaustToolbarNodes = [
          {
            id: 'site-name',
            location: 'primary',
            component: <SiteName url={adminUrl} />,
          },
        ];

        // Removing default site name node to be able to override admin url
        toolbarNodes = [
          ...toolbarNodes.filter((node) => node.id !== 'site-name'),
        ];

        return [...customToolbarNodes, ...toolbarNodes];
      },
    );

there is no way to override rest of the toolbar items to render proper admin urls.

Would be great if we could do something like:

// @note: wpAdminUrl filter is not currently part of @faustwp/core package
hooks.addFilter('wpAdminUrl', 'faust', (url: string) => {
  return url.replace('/wp-admin', '/wp/wp-admin'); // or whatever admin url modification is needed
});

There is a similar request to this one, but it is related to WordPress faustwp plugin https://github.com/wpengine/faustjs/issues/1872#issue-2231682717

ChrisWiegman commented 3 months ago

While @theodesp 's PR, which would solve this in a JavaScript filter for wp-admin in Faust users is excellent, I've asked us both to step back on that for the moment. The issue is that there are more paths than just wp-admin that may need to be accessed directly. As I reviewed his PR I was actually working on #1872 which addresses a different symptom of the same issue with a PHP filter in WordPress. Both would work, but doesn't solve the issue.

The approach we're looking at instead is to expose this via WP GraphQL and I've opened https://github.com/wp-graphql/wp-graphql/issues/3145 to discuss that. I believe this would best help you (so you don't have to ask us for another filter if you have to access something else in the WordPress filesystem) as well as all Faust users and beyond.

justlevine commented 3 months ago

On review, it seems the issue comes from the fact that getAdminUrl() makes two incorrect assumptions over in https://github.com/wpengine/faustjs/blob/2c581ad2fd429bea2bd9385fc65c0391d193b077/packages/faustwp-core/src/lib/getAdminUrl.ts#L10

  1. It conflates the site_url with the home_url (like many parts of Fausts frontend/backend).
  2. It assumes that the admin URI is wp-admin, when it's theoretically possible to move it with some filters and server rewrite rules.

As with https://github.com/wpengine/faustjs/issues/1872#issuecomment-2142713968, I believe the ideal pattern is inheritable constants, and shouldn't require an extra GraphQL request to resolve.

(Fun fact #1360 was actually designed to workaround this exact issue, since /graphql is located on site_url but Faust was forcing it on the home url. There are several other examples of this in both the frontend and in replacements.php)

@ChrisWiegman please confirm/correct my assessment 🙏

jan-clockworkwp commented 1 month ago

@justlevine that is absolutely right, and it took me a while to digest what is going on with the urls in FaustWP plugin and where is the issue coming from, only after realising that site_url() WordPress native method, that is widely used in FaustWp plugin, specially in includes/replacement, can be different (suffixed, based on your setup [Bedrock /wp]) to home_url(), causing that no href, src and url replacement actually work if site_url() is not the same as home_url(), as all Faust plugin rewrites rely on the site_url() but internal links, media and other resources are relying on the actual WordPress front end base url that is equal to home_url(). Probably there are other areas where this incorrect assumption is causing issues as @justlevine pointed out in his comment. As @ChrisWiegman is not with WPE anymore according to https://github.com/wpengine/faustjs/issues/1872#issuecomment-2265114158, @theodesp is there anyone who could take over this kinda feature/bug request? Is there anything I can to do help this one to be merged into the plugin core, and would potentially be solving as well https://github.com/wpengine/faustjs/issues/1872 We are slowly getting closer to production release date and we would love to not go to production with FaustWP plugin patched all over the places. Many thanks for any input.