Stop automatic storage creation of body field for node

I've updated the issue summary clarifying that the creation of the body field should be prevented for content types and block types and added the Needs product manager review per discussion on Slack.

Adding the “Needs documentation updates” tag per Slack discussion with @lawrolan

@sime could you provide a bit more info about what makes this a Starshot blocker?

The change in the merge request and what the title says doesn't really correspond. the body field *storage* is regular default config, what this changes and removes is the automatic addition of the field itself, aka the per-bundle auto-configuration.

@pameela it's not a blocker to starshot contrib efforts, but I think it's at least a soft blocker to core integration. We originally decided to remove text with summary about eight years ago in #2726497: Deprecate text_with_summary but it didn't happen at the time.

To fully deprecate install profiles in core and provide recipe selection in the installer, we need to convert Umami and Standard profiles to recipes. When we convert them to recipes, the constituent parts of those recipes may/should also be re-usable recipes, but then they need to make sense.

Umami was using the text + summary field type despite not using the summary anywhere (as in, none of the content populates the filed, and the special formatter for it is never used), recently changed in #3425105: Don't use text_with_summary in Umami.

Standard also uses text with summary, but it's not a good basis for building a site from (IMO, I always make a completely separate summary field if needed). So if we want the current core install profiles to work nicely together based on common recipes for an article content type etc., we need to clean this up.

edit: #3258492: Change Standard profile to use Media instead of image fields is a similar issue where the core install profiles are using things that have effectively been replaced in core but were never updated.

I've been meeting to revisit but I've tried to remove that body creation feature and default config and it is definitely tricky. Hit failures everywhere.

@catch so wonder if this MR is on the right track?

I've been trying to figure out how to best tackle the deprecation in parts but each seem to open a whole can of worms.

That's how comment module keeping storage but not a field for comment #2422353: Comment module should check that comment body field exists

Wondering if we really want this, because if we just remove this outright then nodes and block content will only have a title by default? I might be wrong but I think users expect both a title and body field if they are creating these from the UI. This is already only done on form submit so if you programmatically create an entity type it doesn't automatically get added.

If we want to stop using "text with summary" can't we just change the default in config/install/field.storage.node.body.yml?

I'm actually not opposed to that idea. 9/10 I'm going to just be adding field_body myself anyway.

@longwave would you mind discussing with other framework managers if that's the desired route? If so that might make this step easier.

So question

Do we want to get the field name has just "body" or "field_body" like we did for Umami? Do we want to change persist_with_no_fields to false while we are at it so those who want can delete the body field.

I think I have misunderstood the issue. This is about preventing the storage from being created automatically, but does not request changes to the UI. So what we probably need to happen is remove core/modules/block_content/config/install/field.storage.block_content.body.yml and core/modules/node/config/install/field.storage.node.body.yml but then fix up the *_add_body_field() methods so that they are capable of creating the storage if it doesn't exist?

This means that minimal installs and recipes will be responsible for managing the storage as required, but manually created block content and content entity types will function exactly the same as before.

So kinda the track the MR is on?

I don't think it's very clear right now what the goal of this issue is. There's a bunch of options with quite different requirements.

1. Deprecation of text with summary field type. As mentioned, we can switch to just a text field without summary and nothing else needs to change. We might want to add a separate teaser/summary field in standard profile for example for article as a replacement though.
2. Starshot is an entirely different situation however. Because " I might be wrong but I think users expect both a title and body field if they are creating these from the UI" is no longer true if you are using an alternative to the body field, for example paragraphs/layout builder/experience builder. At that point, you likely want a body field anymore when you create a node type through the UI, because you won't use it. We remove the body field in almost all cases in our projects as we replace it with a paragraph field. So the answer to that would probably be to move the body fields to standard/umami install profiles and deprecate and no longer call the _add_body_field() functions. Adding new types would then imply that you have to add the body field manually. I assume the field would then also no longer be delete-protected, so if you remove manually from article and page in standard install profile, the storage will be deleted too.
3. comment #20 is something else again and a very different approach than 2.

I agree that we need a clear decision what exactly this issue should accomplish before we start on it. My guess is the focus is Starshot, so that would mean option 2, that is the only option that enables a workflow where creating a new content type in the UI allows to default to experience builder IMHO. But that's a non-trivial shift in behavior that doesn't just affect Starshot but also all existing users of Drupal standard install profile as the UI behavior changes as well as all other distributions that rely on the field.storage.node.body field config being in node module, because if that's moved to standard then their install config is going to be broken.

Because Starshot is using recipes to create content types, we can specify what fields we want. So I don't understand how this affects Starshot at all?

Edit: Actually sorry, of course it affects content types that users create on their own, so I guess it depends whether we decide not to use text_with_summary (which we haven't finalised yet) for our content types. If not, then it would be weird for new content types to have it.

@pameela it's related to starshot in two ways, one is very long term, one not so much.

In the longer term, Drupal core install profile needs to move from install profiles to project browser + recipes. To do this, we need to convert existing install profiles in core to recipes - and those should be based on re-usable components that make sense. e.g. umami should not ship with a text_and_summary field then not use the summary anywhere, which is what it did for years.

Hopefully Umami can be partially re-built on shared foundations with Drupal CMS. Standard might go altogether eventually since it won't really make sense if there is Drupal CMS. Minimal also doesn't really make sense in recipe world either.

In the short term, once Drupal CMS ships with experience builder. If someone creates a new content type, that content type should not come with a 'body' field using text + summary because that would be very confusing if XB is also enabled by default (or even if it's not, because you'd have to both enable XB and delete a field).

@berdir fwiw I think both the reasons in #22 are good:

1. I don't like the text and summary field and I think that is quite common, it could live in contrib.
2. The automatic creation of the body field also seems anachronistic.

However we can and should separate those things if it makes it easier to get done.

Ah yes the XB part makes sense, I was only thinking of Starshot v1 which probably won’t be affected.

Created #3477043: Change automatic body field creation to use formatted text field instead of text_with_summary to unblock Starshot while we figure this out.

Maybe we don't need this part since #3482259: Landing page integration: new content entity type for unstructured content (creating a custom entity as the default for XB) is the plan for the moment? Or we can postpone it to see what happens there?

Thinking more about it, I guess it’s unlikely that any node type using XB would want a body field specifically so I don’t think it does change things.

No longer a blocker since #3488742: Stop calling node_add_body_field() from NodeTypeForm landed.

Closing as a dup since #3488742: Stop calling node_add_body_field() from NodeTypeForm landed

Actually nvm think they just overlapped, but lets postpone on #3489266: Deprecate node_add_body_field()

#3489266: Deprecate node_add_body_field() is in.

#3488742: Stop calling node_add_body_field() from NodeTypeForm was the user-facing change, so removing the product manager review tag from this one.

So what should the scope of this issue be? Config cleanup?

Removing the default config file. And then adjust the places that do rely on it, such as the deprecated but still expected to function node_add_body_field() to create the storage on the fly if missing.

Also, when removing it from node module, it needs to be added to standard/umami and recipes that create node types with a body field.

You mean remove the storage.body file?

Yes, but also moving it to places that need it like profiles and recipes

Going to give it a shot!

Actually another question do we want to change the storage body file to be text formatted not text formatted with summary since the goal is to deprecate that plugin?

I don't think so, not in this issue. lets move it out of node.module first I' say. going to be tricky enough a it is.

Actually postponed on #3447617: Stop automatic storage creation of body field for node which I already have included in this MR.

Could use some eyes though

I'm really confused by #43, where this issue appears to be postponed upon itself. :recursion: 🤣

Maybe you meant #3489266: Deprecate node_add_body_field()? But that one is already committed + fixed. So I believe this only needs a rebase to latest 11.x branch.

Oops I meant postponed on https://www.drupal.org/project/drupal/issues/3535526

I don't think it needs to be deprecated on that. There is no connection between the two other than being the same concept.

I'd do the config removal either there or in a separate issue for that.

If the block content add body ticket doesn't land in the next day or two I'll untangle these two.

Okay have pulled out the storage for block_content and just focusing on node now.

Failure is unrelated #3539331: Skip failing test with incorrect warning for system requirements about APCu memory

Posted a review. And because my comments surely weren't annoying enough yet, I'll finish up with a bonus: The node:body and node:summary tokens. What on earth are we going to do with those? Deprecate them? See also #2885825: "summary" token for text+summary field is empty when using anything other than the core "body" field, metatag relies on this as default configuration.

Regardless if we keep or not node:summary would have to be deprecated I imagine. If we kept the storage.body and changed it to a storage not text_with_summary.

So my current question is this which is preferred

1. We keep storage.body in the node module and change the type to not use summary.
2. We remove it from node, which current MR is doing

The node:body and node:summary tokens. What on earth are we going to do with those? Deprecate them?

I think that we can and should deprecate these. Not sure exactly how though. We might need to leave the tokens in and trigger a deprecation message for a long time? We already render an empty string if the field isn't on the node type per #1671420: If no body field, [node:summary] [node:body] etc tokens are not replaced and metatag relying on this seems like a bug, it could accept a special view mode (like search indexing does) or similar maybe?

We keep storage.body in the node module and change the type to not use summary.

I personally think the end point should be that node module doesn't ship the storage field any more.

pipeline is green but not sure how to use node_storage_body_field

And with regards to the summary token what if we move that to the text_with_summary module we eventually create?

not sure how to use node_storage_body_field

We shouldn't be relying on it at all in core. The most we could do is add a test to make sure it works as a bc layer - e.g. a test module that ships a body field that depends on field storage body and also the module. Install the test module, make sure the field is created, uninstall field_storage_body, make sure the fields are still there after install - because we want people to uninstall the deprecated module but also keep their fields.

Metatag already depends on the token module so maybe the summary token could be moved there? Token module should also already provide a direct replacement for node:body with the entity field chaining stuff it has.

Believe this one is ready now

With regards to the token. Think after this one lands I’ll start the text_with_summary module and maybe we can move the summary token there?

Drafted a CR https://www.drupal.org/node/3540814 and made the storage module hidden.

I think the module will need to hook_system_info_alter() to mark itself as un-hidden when it's installed, so that people can uninstall it. That hopefully should work pretty well though.

Added!

I think this is much closer than the number of comments I added would normally imply.

A couple are just to make sure I didn't miss something.
Several are related to persisting storage (I think we want to keep the profile one, but the rest I think should be false)
One potentially complicated related to migrate, but I think it's handled already.

CR I think should include the file name and content and a brief explanation on when you would choose persist (or at least a link to docs on that option) https://api.drupal.org/api/drupal/core%21modules%21field%21src%21Entity%...

Tried to address some of the feedback let me know your thoughts!

Reverted the change to the testing profile and article recipe as it broke tests. Probably could be a follow up to stop persisting the storage?

Addressed latest feedback

I think most feedback has been addressed and this is almost ready.

There are however a couple of things:

1 I think berdir needs to confirm you addressed his concern. I think you did, but his concern was pretty sweeping. Maybe ping @berdir and t@hejimbirch in slack.
2 you mentioned a request to split out the create in response to @larowlan but I can't find the comment you're talking about.

Figured out @berdirs concerns, I left more breadcrumbs in the MR feel free to ping me on slack or here for clarification.

Opened #3547530: Can you specify non existing config in a recipe and #3547531: BC concern for config in recipes

All feedback has been addressed, I think this is a great step!

Some merge conflicts in this one.

Fixed the test conflicts.

Couple of comments but should be pretty small.

Withdrawing my comment about the submodule because that's what layout_builder did per @smustgrave in slack. Given the module only exists in order to be deprecated I think it's fine then, just didn't want to set a precedent.

Updated description.

OK this and related issues have turned out to be incredibly tricky but good to get an extra step towards removing this (and text_with_summary).

Committed/pushed to 11.x, thanks!

FYI this left us in a situation where the [node:summary] token no longer outputs anything, so I opened a new issue to discuss it: #3553018: node:summary and node:body tokens rely on field removed in 11.3+

Now this change is problematic for modules that ship with their own node type and fields configuration, if they want to support multiple Drupal versions.

Scenario

To be more specific, we have a module "my_content_news" with:
- config/install/node.type.my_news.yml
- config/install/field.field.node.my_news.body.yml

(I know that recipes exist, but they are not a good option if we want to provide an upgrade path)

For Drupal <= 11.2, this module would simply depend on node, and get the field.storage.node.body from there.
This would break with Drupal >= 11.3, because field.storage.node.body does not exist.

For Drupal >= 11.3, it needs to depend on node_storage_body_field.
This would break with Drupal <= 11.2, because node_storage_body_field module does not exist.

From what I know, there is no mechanism for a module to have different dependencies depending on core version.

Solutions

One solution would be a "polyfill" contrib module "node_storage_body_field", which would just be empty.
But should a non-major core update require a polyfill?

Another option might be to create the field base in hook_install(). But I don't really like that.

A third option is to release different versions of my_content_news for Drupal <= 11.2 and Drupal >= 11.3.
But now we would have to maintain two major versions, for a non-major core version upgrade.

Another option is to move all config to optional, then it will silently ignore config that already exists

Another option is to move all config to optional, then it will silently ignore config that already exists

This actually works, we can simply copy the field storage into config/optional.
We would end up with multiple modules that have their own copy of that field storage.

Btw it only works if we also move the field instances into config/optional.

The change record already says this:

But it's advised to just ship with your own copy of field.storage.node.body.yml.

So if we mention that you can or should use config/optional/ for this, we are good.

Another option is to move all config to optional, then it will silently ignore config that already exists

Actually this is not a good solution for an existing module, because:

• I think we lose the config dependency validation on module install.
• All dependent configurations in this and other modules also need to move to config/optional.

We might still do it if we find no other way, but it is clearly more disruptive than it should be.
For us it means changes across multiple packages, with yet unclear side effects.

I might actually explore the polyfill solution.

We created https://www.drupal.org/project/backport_node_storage_body_field as a backport / polyfill to solve this problem.