refactor: adjust Frontend to new Editor storage format #4465
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
📦 Next.js Bundle Analysis for @serlo/frontendThis analysis was generated by the Next.js Bundle Analysis action. 🤖 🎉 Global Bundle Size Decreased
DetailsThe global bundle is the javascript bundle that loads alongside every page. It is in its own category because its impact is much higher - an increase to its size means that every page on your website loads slower, and a decrease means every page loads faster. Any third party scripts you have added directly to your app using the If you want further insight into what is behind the changes, give @next/bundle-analyzer a try! Seven Pages Changed SizeThe following pages changed size from the code in this PR compared to its base branch:
DetailsOnly the gzipped size is provided here based on an expert tip. First Load is the size of the global bundle plus the bundle for the individual page. If a user were to show up to your website and land on a given page, the first load size represents the amount of javascript that user would need to download. If Any third party scripts you have added directly to your app using the Next to the size is how much the size has increased or decreased compared with the base branch of this PR. If this percentage has increased by 20% or more, there will be a red status indicator applied, indicating that special attention should be given to this. |
| }) | ||
| } | ||
|
|
||
| return { editorMetadata, templateContent } |
There was a problem hiding this comment.
Why is there template in the name?
There was a problem hiding this comment.
Because it's representing the content of TemplatePluginType. Would templatePluginContent be more understandable?
There was a problem hiding this comment.
Ah, I see. How about just content? Or is this function only for TemplatePluginType?
There was a problem hiding this comment.
Or is this function only for TemplatePluginType?
Yes. So the editor metadata will be in its content property. But the editor's migrate function isn't expecting that. More detail here.
| ...createEmptyDocument('serlo-org'), | ||
| document: { | ||
| plugin: AllowedPlugins[entityType], | ||
| }, |
There was a problem hiding this comment.
Maybe createEmptyDocument could generate different initial editor states in the future. Example: createEmptyDocument(editorVariant: 'serlo-org', type: 'article')
There was a problem hiding this comment.
If you don't mind, could you please explain the benefits of that in more detail? Is it to avoid this destructuring approach, or are there other benefits?
There was a problem hiding this comment.
When I created this function in the past there was only one root plugin type-generic-content so it made sense to hard code it. Now, there are different types. I think I would slightly prefer having all that logic in function createEmptyDocument. But the way it is now is also fine for me.
There was a problem hiding this comment.
Fair point! And it should stay like that, actually. I only exported the createEmptyDocument function through the package as a special case for Serlo (which is also documented by comments in the package index file, and also why it's not mentioned in the README). So it's a temporary hacky solution, and I wouldn't change the original function for it.
It's definitely not great, but I decided that these refactorings need to go step by step, if I tried to make everything perfect on the first go, I'd get lost in it 😁
Linear: EDTR-98
API (db-migrations) PR: serlo/api.serlo.org#1865
Infra PR: serlo/infra#120
Should only be merged once the DB migration has been run on staging.