RFC: Payload Admin Components Abstraction #6938
Replies: 13 comments 10 replies
-
The whole point of ESM and moving to next was to reduce the complexity. As long as we aren't losing any actual functionality, I see no reason as to why this shouldn't be done. Maintaing a custom loader? The way I see it, if we can make that burden a little easier on you guys, that is more resources towards actually working on features we can all touch. |
Beta Was this translation helpful? Give feedback.
-
I would accept the DX tradeoffs, though I don't think it changes much for our team. Type conflicts for components would still be reported after a change / save, just not automatically right? Also, curious, why do people import their payload configuration into the frontend? |
Beta Was this translation helpful? Give feedback.
-
Why not separate base config into client config + server config? This way importing only server config for strictly server things would not load anything to client |
Beta Was this translation helpful? Give feedback.
-
To me it feels like we're passing solvable, technical limitations onto the end user. My biggest fear is that static paths won't automatically update as components are renamed and moved around, unlike import paths do. That would be a serious pain. |
Beta Was this translation helpful? Give feedback.
-
I'm asking this as someone who's not exactly a power user of payload and also a first-timer on NextJS (which is prolly a non-insignificant chunk of Payload's userbase), but if I'm understanding this correctly, as long as I don't use custom fields, it changes nothing for me? Or does this apply for any component included in admin panel using payload.config (like the seeding component that shows up in website template)? Former seems a non-issue whereas latter could be a little complicated to a noob like me (or maybe not if that's how the examples were set up to begin with?). |
Beta Was this translation helpful? Give feedback.
-
Disclaimer: I only picked up Payload yesterday and don't know much about how the code splitting in Next.js or Payload works. |
Beta Was this translation helpful? Give feedback.
-
Sounds alright, decent trade off. The big downside to me is these paths not being validated and automatically refactored by my IDE. I can already see some bugs on the horizon caused by this. Perhaps it is possible to get a build time error at least, for an invalid import path? That is; a path that does not point to a file with the default export being a JS function or some similar assertion. |
Beta Was this translation helpful? Give feedback.
-
In 1 of my plugins I'm doing a strange, yet needed thing https://github.com/r1tsuu/payload-enchants/blob/df706ed8e66a4b5dbb75bba1d4b3e121aa20cf3f/packages/better-localized-fields/src/attachLocalizedField.tsx#L52 Honestly in Payload 3.0 i think we already lost kinda important thing - we can't just pass functions from config to components, we need to execute them via REST / Server actions, meaning that any dynamic client-side logic, like:
requires additional request, but i see that Real Time API could possibly solve this. Have you guys explored for a simple solution to this? Also it can be nightmare to manage a lot of files. While for fully featured custom fields you normally have a separated file, imagine you have 10 fields and in each field you want a custom, 2 lined description component. ATM you can just create it inline and it feels good, but maybe I'm just not a fan of having a lot of files for 2 line things. |
Beta Was this translation helpful? Give feedback.
-
Discplaimer:
It wouldnt require maintaining 2 separate configs, just carefuly choosing which one of these we import, and to me that sounds like a fair tradeoff. |
Beta Was this translation helpful? Give feedback.
-
Hi @jmikrut, I share the same concern as @r1tsuu pointed out ! Though I haven't worked with Payload yet (waiting for the stable 3.0 release), I wanted to share my opinion. How would you handle scenarios in the Payload config where you want to do props drilling into your components ? Or even handle conditional rendering without making additional requests on the components themselves ? I might be missing something, but in my opinion, we would lose a lot of flexibility (in addition to the IDE checks and refactoring issues you already mentioned) in the way we're handling custom Admin components. Can you provide an alternative to @r1tsuu's implementation that would work with the new approach ? Also, adding additional build/runtime checks would impact build/hot-reload times, right ? |
Beta Was this translation helpful? Give feedback.
-
I'd be fine with this approach provided that Payload threw some proactive errors (when custom components weren't found) It seems to decrease the surface area for bugs in other areas and provides a much needed perf improvement for the 90% user base with a little friction for the 10% using custom fields (don't quote me on those numbers, completely fictional but you get the idea) |
Beta Was this translation helpful? Give feedback.
-
As i understood this concern more payload dashboard, for me i would prefer to add my component directly, the path can be misleading especially without errors, so yeah throw us errors and will be fine |
Beta Was this translation helpful? Give feedback.
-
I like the single config file approach Payload uses so far. This makes it simple to write plugins which just have to modify this single config. However, I reported the issue with Lexical CSS code being added to the Next.js page on Payload 2.x quite some time ago. Separating client/server code is non-trivial and outside of Next.js different toolchains are involved (Next.js still has at least two; with Babel three). In our projects we use a single config file for the server and during build time we create the sanitized client configuration file. Probably such a pre-processor could run once while creating the TypeScript and GraphQL files. I know you already do some tricks while loading the current config. Concerning Next.js, there are some difficulties, like the Edge runtime which easily breaks if not supported Node.js packages are imported. Edge might be replaced by a normal Node.js runtime but haven't seen any announcements yet. Using the instrumentation feature we had to set externals in the Weback config to exclude fsevents which uses native code. Next.js pre-bundles the server code and has some restrictions. Like @r1tsuu, we are using closures to pass props to components: components: {
Field: createTaxonomySummary(parentCollection, taxonomy)
}
/**
* Create a TaxonomySummary instance.
*
* @param parentCollection
* @param taxonomy
* @returns
*/
export function createTaxonomySummary(parentCollection: CollectionConfig, taxonomy: string) {
return function TaxonomySummaryCollection() {
return (
<TaxonomySummary
parentCollection={parentCollection}
taxonomy={taxonomy}
/>
);
};
} With strings this will no longer work. One workaround could be to move the taxonomy value to the custom property and get the collection via hooks. I hope there is a way to pre-process the config to keep its flexibility and generate ready to use configs for all purposes. |
Beta Was this translation helpful? Give feedback.
-
Hi everyone, first up, YOU ARE ALL GREAT. You have been so extremely helpful while we build toward 3.0 - and we are VERY close to stable. There are only a few things left. One item on our radar to solve for yet is something I'd like to discuss here.
Many of you likely already know that we have a pull request open to Next.js in regards to being able to use Payload's Local API in your own frontends / React Server Components.
Click the issue to read more about the challenge. It's a tough one, and technically it's not really something that Next.js is doing wrong. Next.js is just simply making sure that anything we import (and use) gets bundled up to the client-side JS for a page.
Right now, you might have the following client-side JS in a Payload config:
This happens because you actually import custom admin UI components into your config, and bind them to the config accordingly.
Here's an example:
So the problem is pretty much the fact that we allow you to import and pass React components to the config, which is great from a DX perspective.
But then, if you import your Payload config in your frontend, all these components get sent to your client-side JS, which means you might have another few hundred + KBs of first-load JS right out of the box, just by importing the Payload config.
We can't let that happen. And we've gone down a LOT of paths to find out how we can solve this problem (my Next.js PR being one solution).
But while my PR does indeed solve the issue, there is a more technically elegant solution available.
This all has to do with the
import
keyword.Alternative solution
If we instead accepted a string-based path to your component, we could only load those components in the admin UI, and the Payload config would get smaller / slimmer. Like this:
Pros of this approach:
Another pretty large pro to this approach is that right now, Payload has to maintain a custom "loader" that can safely import the Payload config if you are using it in standalone scripts, or outside of bundler contexts. We have to maintain this loader because we need to "disregard" client-side SCSS / CSS / JPG / SVG / etc imports, because those would crash vanilla Node.
We use this "loader" for Payload bin scripts (
payload generate:types
/ etc.)Our loader is getting more bulletproof, and we've always needed to provide this, but it's been the source of bugs as we've worked through the transition to ESM / 3.0. If we didn't allow client files / React components to be directly imported, and instead we just accepted a string path, we could simplify here.
Overall though, this change would make the
payload
package and your Payload config significantly more portable / re-usable outside of Next.js.Cons of this approach:
default
payload.init
which builds out a "map" file that the admin UI imports, which contains all custom componentsConclusion
We are not sprinting toward this being our recommendation. We are still working with the Next.js team on a solution that would allow us to continue to import client-side JS in our Payload config, but I do keep having this idea pop back into my head as the team and I think through solutions here.
It's possible that even outside of Next.js, this approach would have benefits that would make it valuable anyway.
But that's why I'm here writing this - I want to hear from you. What do you think? Would you accept the DX tradeoffs of passing a string path to your component, vs. passing the component itself to the Payload config?
Beta Was this translation helpful? Give feedback.
All reactions