The introduction to config_split_state is bloating views config [#3236393]

Problem/Motivation

There seems to be a lot of bulk being added to config that is not owned by config_split

Steps to reproduce

After upgrading to 2.0.0-beta4 I saved a small change to an exposed filter to make it not hierarchy: false but instead I got this:

diff --git a/config/sync/views.view.projects.yml b/config/sync/views.view.projects.yml
index 4d82e83..09e7cb9 100644
--- a/config/sync/views.view.projects.yml
+++ b/config/sync/views.view.projects.yml
@@ -350,7 +350,7 @@ display:
           type: select
           limit: true
           vid: project_type
-          hierarchy: true
+          hierarchy: false
           error_message: true
           plugin_id: taxonomy_index_tid
       sorts:
@@ -429,6 +429,35 @@ display:
         - 'config:core.entity_view_display.node.video.teaser'
         - 'config:field.storage.node.field_project_status'
         - 'config:field.storage.node.field_project_type'
+        - 'config_split_state:core.entity_view_display.node.article.default'
+        - 'config_split_state:core.entity_view_display.node.article.rss'
+        - 'config_split_state:core.entity_view_display.node.article.short_teaser'
+        - 'config_split_state:core.entity_view_display.node.article.teaser'
+        - 'config_split_state:core.entity_view_display.node.carousel_pane.default'
+        - 'config_split_state:core.entity_view_display.node.carousel_pane.teaser'
+        - 'config_split_state:core.entity_view_display.node.documents.default'
+        - 'config_split_state:core.entity_view_display.node.documents.teaser'
+        - 'config_split_state:core.entity_view_display.node.event.default'
+        - 'config_split_state:core.entity_view_display.node.event.short_teaser'
+        - 'config_split_state:core.entity_view_display.node.event.teaser'
+        - 'config_split_state:core.entity_view_display.node.kmob.default'
+        - 'config_split_state:core.entity_view_display.node.kmob.teaser'
+        - 'config_split_state:core.entity_view_display.node.organization.default'
+        - 'config_split_state:core.entity_view_display.node.organization.teaser'
+        - 'config_split_state:core.entity_view_display.node.page.default'
+        - 'config_split_state:core.entity_view_display.node.page.teaser'
+        - 'config_split_state:core.entity_view_display.node.person.default'
+        - 'config_split_state:core.entity_view_display.node.person.short_teaser'
+        - 'config_split_state:core.entity_view_display.node.person.teaser'
+        - 'config_split_state:core.entity_view_display.node.person_trainee.default'
+        - 'config_split_state:core.entity_view_display.node.person_trainee.short_teaser'
+        - 'config_split_state:core.entity_view_display.node.person_trainee.teaser'
+        - 'config_split_state:core.entity_view_display.node.project.default'
+        - 'config_split_state:core.entity_view_display.node.project.teaser'
+        - 'config_split_state:core.entity_view_display.node.video.default'
+        - 'config_split_state:core.entity_view_display.node.video.teaser'
+        - 'config_split_state:field.storage.node.field_project_status'
+        - 'config_split_state:field.storage.node.field_project_type'
   attachment_1:
     display_plugin: attachment
     id: attachment_1
@@ -638,6 +667,35 @@ display:
         - 'config:core.entity_view_display.node.video.teaser'
         - 'config:field.storage.node.field_project_status'
         - 'config:field.storage.node.field_project_type'
+        - 'config_split_state:core.entity_view_display.node.article.default'
+        - 'config_split_state:core.entity_view_display.node.article.rss'
+        - 'config_split_state:core.entity_view_display.node.article.short_teaser'
+        - 'config_split_state:core.entity_view_display.node.article.teaser'
+        - 'config_split_state:core.entity_view_display.node.carousel_pane.default'
+        - 'config_split_state:core.entity_view_display.node.carousel_pane.teaser'
+        - 'config_split_state:core.entity_view_display.node.documents.default'
+        - 'config_split_state:core.entity_view_display.node.documents.teaser'
+        - 'config_split_state:core.entity_view_display.node.event.default'
+        - 'config_split_state:core.entity_view_display.node.event.short_teaser'
+        - 'config_split_state:core.entity_view_display.node.event.teaser'
+        - 'config_split_state:core.entity_view_display.node.kmob.default'
+        - 'config_split_state:core.entity_view_display.node.kmob.teaser'
+        - 'config_split_state:core.entity_view_display.node.organization.default'
+        - 'config_split_state:core.entity_view_display.node.organization.teaser'
+        - 'config_split_state:core.entity_view_display.node.page.default'
+        - 'config_split_state:core.entity_view_display.node.page.teaser'
+        - 'config_split_state:core.entity_view_display.node.person.default'
+        - 'config_split_state:core.entity_view_display.node.person.short_teaser'
+        - 'config_split_state:core.entity_view_display.node.person.teaser'
+        - 'config_split_state:core.entity_view_display.node.person_trainee.default'
+        - 'config_split_state:core.entity_view_display.node.person_trainee.short_teaser'
+        - 'config_split_state:core.entity_view_display.node.person_trainee.teaser'
+        - 'config_split_state:core.entity_view_display.node.project.default'
+        - 'config_split_state:core.entity_view_display.node.project.teaser'
+        - 'config_split_state:core.entity_view_display.node.video.default'
+        - 'config_split_state:core.entity_view_display.node.video.teaser'
+        - 'config_split_state:field.storage.node.field_project_status'
+        - 'config_split_state:field.storage.node.field_project_type'
   page_1:
     display_plugin: page
     id: page_1
@@ -730,6 +788,35 @@ display:
         - 'config:core.entity_view_display.node.video.teaser'
         - 'config:field.storage.node.field_project_status'
         - 'config:field.storage.node.field_project_type'
+        - 'config_split_state:core.entity_view_display.node.article.default'
+        - 'config_split_state:core.entity_view_display.node.article.rss'
+        - 'config_split_state:core.entity_view_display.node.article.short_teaser'
+        - 'config_split_state:core.entity_view_display.node.article.teaser'
+        - 'config_split_state:core.entity_view_display.node.carousel_pane.default'
+        - 'config_split_state:core.entity_view_display.node.carousel_pane.teaser'
+        - 'config_split_state:core.entity_view_display.node.documents.default'
+        - 'config_split_state:core.entity_view_display.node.documents.teaser'
+        - 'config_split_state:core.entity_view_display.node.event.default'
+        - 'config_split_state:core.entity_view_display.node.event.short_teaser'
+        - 'config_split_state:core.entity_view_display.node.event.teaser'
+        - 'config_split_state:core.entity_view_display.node.kmob.default'
+        - 'config_split_state:core.entity_view_display.node.kmob.teaser'
+        - 'config_split_state:core.entity_view_display.node.organization.default'
+        - 'config_split_state:core.entity_view_display.node.organization.teaser'
+        - 'config_split_state:core.entity_view_display.node.page.default'
+        - 'config_split_state:core.entity_view_display.node.page.teaser'
+        - 'config_split_state:core.entity_view_display.node.person.default'
+        - 'config_split_state:core.entity_view_display.node.person.short_teaser'
+        - 'config_split_state:core.entity_view_display.node.person.teaser'
+        - 'config_split_state:core.entity_view_display.node.person_trainee.default'
+        - 'config_split_state:core.entity_view_display.node.person_trainee.short_teaser'
+        - 'config_split_state:core.entity_view_display.node.person_trainee.teaser'
+        - 'config_split_state:core.entity_view_display.node.project.default'
+        - 'config_split_state:core.entity_view_display.node.project.teaser'
+        - 'config_split_state:core.entity_view_display.node.video.default'
+        - 'config_split_state:core.entity_view_display.node.video.teaser'
+        - 'config_split_state:field.storage.node.field_project_status'
+        - 'config_split_state:field.storage.node.field_project_type'
   page_2:
     display_plugin: page
     id: page_2
@@ -867,3 +954,32 @@ display:
         - 'config:core.entity_view_display.node.video.teaser'
         - 'config:field.storage.node.field_project_status'
         - 'config:field.storage.node.field_project_type'
+        - 'config_split_state:core.entity_view_display.node.article.default'
+        - 'config_split_state:core.entity_view_display.node.article.rss'
+        - 'config_split_state:core.entity_view_display.node.article.short_teaser'
+        - 'config_split_state:core.entity_view_display.node.article.teaser'
+        - 'config_split_state:core.entity_view_display.node.carousel_pane.default'
+        - 'config_split_state:core.entity_view_display.node.carousel_pane.teaser'
+        - 'config_split_state:core.entity_view_display.node.documents.default'
+        - 'config_split_state:core.entity_view_display.node.documents.teaser'
+        - 'config_split_state:core.entity_view_display.node.event.default'
+        - 'config_split_state:core.entity_view_display.node.event.short_teaser'
+        - 'config_split_state:core.entity_view_display.node.event.teaser'
+        - 'config_split_state:core.entity_view_display.node.kmob.default'
+        - 'config_split_state:core.entity_view_display.node.kmob.teaser'
+        - 'config_split_state:core.entity_view_display.node.organization.default'
+        - 'config_split_state:core.entity_view_display.node.organization.teaser'
+        - 'config_split_state:core.entity_view_display.node.page.default'
+        - 'config_split_state:core.entity_view_display.node.page.teaser'
+        - 'config_split_state:core.entity_view_display.node.person.default'
+        - 'config_split_state:core.entity_view_display.node.person.short_teaser'
+        - 'config_split_state:core.entity_view_display.node.person.teaser'
+        - 'config_split_state:core.entity_view_display.node.person_trainee.default'
+        - 'config_split_state:core.entity_view_display.node.person_trainee.short_teaser'
+        - 'config_split_state:core.entity_view_display.node.person_trainee.teaser'
+        - 'config_split_state:core.entity_view_display.node.project.default'
+        - 'config_split_state:core.entity_view_display.node.project.teaser'
+        - 'config_split_state:core.entity_view_display.node.video.default'
+        - 'config_split_state:core.entity_view_display.node.video.teaser'
+        - 'config_split_state:field.storage.node.field_project_status'
+        - 'config_split_state:field.storage.node.field_project_type'

Proposed resolution

TBD, maybe remove and re-think this feature?

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork config_split-3236393

Show commands

Start within a Git clone of the project using the version control instructions.

Add & fetch this issue fork’s repository

Or, if you do not have SSH keys set up on git.drupalcode.org:

Add & fetch this issue fork’s repository

3236393-the-introduction-to changes, plain diff MR !12
Check out this branch for the first time

Check out existing branch, if you already have it locally

About issue forks

Comments

Comment #1

17 September 2021 at 18:20

joelpittet created an issue. See original summary.

Comment #2

joelpittet

he/him

English

Vancouver

commented 17 September 2021 at 18:27

Version:

2.0.0-beta4

» 2.0.x-dev

A quick check and the 2.0.x-dev branch is in sync with beta4 at the moment.

The config_split_state seems to have been introduced in beta3 in the new StatusOverride class.

Comment #3

bob.hinrichs commented 17 September 2021 at 19:04

I posted a different issue in a comment, nevermind...

Comment #4

bircher

🇨🇿

commented 17 September 2021 at 22:29

if I had to guess I would say that it is a bug in https://git.drupalcode.org/project/config_split/-/blob/2.0.x/src/Config/...

Maybe we just need to check there that the name is overwritten before adding cache metadata for it.

Comment #5

18 September 2021 at 00:28

joelpittet opened merge request !12

Comment #6

joelpittet

he/him

English

Vancouver

commented 18 September 2021 at 00:31

Status:

Active

» Needs review

@bircher, thanks for the hint! That seems to help quite a bit, see MR, hope that is what you were suggesting in #4?

But it still has this extra output which I'm not sure if it's needed (and would prefer not to have in views):

@@ -429,6 +429,8 @@ display:
         - 'config:core.entity_view_display.node.video.teaser'
         - 'config:field.storage.node.field_project_status'
         - 'config:field.storage.node.field_project_type'
+        - 'config_split_state:field.storage.node.field_project_status'
+        - 'config_split_state:field.storage.node.field_project_type'
   attachment_1:
     display_plugin: attachment
     id: attachment_1
@@ -638,6 +640,8 @@ display:
         - 'config:core.entity_view_display.node.video.teaser'
         - 'config:field.storage.node.field_project_status'
         - 'config:field.storage.node.field_project_type'
+        - 'config_split_state:field.storage.node.field_project_status'
+        - 'config_split_state:field.storage.node.field_project_type'
   page_1:
     display_plugin: page
     id: page_1
@@ -730,6 +734,8 @@ display:
         - 'config:core.entity_view_display.node.video.teaser'
         - 'config:field.storage.node.field_project_status'
         - 'config:field.storage.node.field_project_type'
+        - 'config_split_state:field.storage.node.field_project_status'
+        - 'config_split_state:field.storage.node.field_project_type'
   page_2:
     display_plugin: page
     id: page_2
@@ -867,3 +873,5 @@ display:
         - 'config:core.entity_view_display.node.video.teaser'
         - 'config:field.storage.node.field_project_status'
         - 'config:field.storage.node.field_project_type'
+        - 'config_split_state:field.storage.node.field_project_status'
+        - 'config_split_state:field.storage.node.field_project_type'

Comment #7

bircher

🇨🇿

commented 18 September 2021 at 06:39

yes something along those lines.

I don't know what is still causing that or what your view is showing. Maybe we need to check that it is a config_split config in more places because this is meant to just override the splits.

Comment #8

joelpittet

he/him

English

Vancouver

commented 18 September 2021 at 21:48

@bircher with simplytest.me I installed beta4, created a test View with node and fields display and added the image field and saved that. Then I get this in my export config:

   cache_metadata:
      max-age: -1
      contexts:
        - 'languages:language_content'
        - 'languages:language_interface'
        - url.query_args
        - 'user.node_grants:view'
        - user.permissions
      tags:
        - 'config:field.storage.node.field_image'
        - 'config_split_state:field.storage.node.field_image'

Didn't see it without "fields" row type (tried just saving Content overview default view. So might only be related to views with Fields row type

Comment #9

bircher

🇨🇿

commented 19 September 2021 at 06:21

Status:

Needs review

» Needs work

Ok so this confirms my suspicion. I naively thought the method on my override would only be called for things I override. But we have to check the name and see if it starts with config_split.config_split. and return the empty metadata if it doesn't (or NULL if the interface allows that, need to check that when back on a laptop)

Comment #10

bircher

🇨🇿

commented 19 September 2021 at 06:24

while we are at it we could probably also remove the extra tag and just use the normal config one sine we also invalidate it when the override state changes.

Comment #11

bircher

🇨🇿

commented 19 September 2021 at 06:25

so actually.. we don't need to provide any extra metadata no?

Comment #12

bircher

🇨🇿

commented 19 September 2021 at 18:38

Status:

Needs work

» Needs review

lets see if that worked from the phone.

Comment #13

joelpittet

he/him

English

Vancouver

commented 19 September 2021 at 23:49

Status:

Needs review

» Reviewed & tested by the community

Yup that did the trick, did you want to still do the code to check for config_split.config_split?

Side note: if anybody exported changes with this in, it will persist until removed manually and imported. (I tried just saving the views again and exporting, but the tags still persisted until I removed manually, imported, re-saved views, and exported)

Comment #14

20 September 2021 at 05:04

bircher committed 3d4f7d9 on 2.0.x authored by joelpittet

Issue #3236393 by joelpittet, bircher: Do not add unnecessary cache tags...

Comment #15

bircher

🇨🇿

commented 20 September 2021 at 05:06

Status:

Reviewed & tested by the community

» Fixed

Ok great!
No need to do anything for now! Thanks for the collaboration.

Comment #16

4 October 2021 at 05:09

Status:

Fixed

» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.

The introduction to config_split_state is bloating views config

Problem/Motivation

Steps to reproduce

Proposed resolution

Remaining tasks

User interface changes

API changes

Data model changes

Issue fork config_split-3236393

Comments

Comment #1

Comment #2

Comment #3

Comment #4

Comment #5

Comment #6

Comment #7

Comment #8

Comment #9

Comment #10

Comment #11

Comment #12

Comment #13

Comment #14

Comment #15

Comment #16

News items

Our community

Documentation

Drupal code base

Governance of community