verbb / super-table

Super-charge your Craft workflow using Super Table.
MIT License
317 stars 47 forks source link

Saving a Super Table field results in a lot of noise in the project config #500

Open MoritzLost opened 1 year ago

MoritzLost commented 1 year ago

Describe the bug

Every time a Super Table field is saved, the field generates a new random changedFieldIndicator value in the project config, even if nothing about the field has changed. This results in a lot of noise in git. It's especially annoying for Super Table fields inside Matrix fields. Ever time the Matrix field is saved, all Super Table fields inside the Matrix field get a new changedFieldIndicator, even if those fields weren't changed at all. The config for the matrix blocks that contain the Super Table fields then get a changedFieldIndicator as well. So saving a Matrix field with two blocks that contain Super Table fields always touches at least five files, even if nothing at all was changed:

Screenshot of the git status view showing multiple edited files

This is very inconvenient, since it creates noise in git and results in a lot of unnecessary merge conflicts. Of course, you can selectively reset those files and not commit them, but since they are changed every time the field is saved, this is really tedious.

The field should check if any changes were made to the config, and only generate a new changedFieldIndicator if there actually are any changes.

Steps to reproduce

  1. Note the changedFieldIndicator in the yml file for any Super Table field inside config/project/superTableBlockTypes.
  2. Save the field in the backend (without changing any settings).
  3. Check the yml file again, it will contain a new changedFieldIndicator. This value will change every time the field is saved.

Craft CMS version

4.3.6.1

Plugin version

3.0.7

Multi-site?

No response

Additional context

No response

darylknight commented 1 year ago

I just discovered this is Super Table, not Craft causing this behaviour - I was thrown off by it always happening in matrix fields. See https://github.com/craftcms/cms/discussions/12597 for info. Like the original post above says; every time you save a field in Super Table it generates dozens of changed config files which creates a lot of unnecessary fluff in the commit. It'd be great if this could be reduced to just the field that was changed.

darylknight commented 1 year ago

Pretty much all of these changed files are because Super Table has updated the changedFieldIndiciator on every block inside a large Matrix field:

image

MoritzLost commented 1 year ago

@engram-design Could we get a comment on this? Would be nice to know if and when this might be fixed. This is one of the largest issue we're facing in our workflow right now, and any git-based workflow will have the same problem.

Looks like the changedFieldIndicator is a workaround for a bug regarding nested Matrix and Super Table blocks. If this workaround is still necessary, maybe it can be improved to update change the changedFieldIndicator when anything has actually changed? For example, by comparing the previous settings to the current settings and only update the indicator when they differ? Or even just limit this indicator to the specific case it's meant to address (Nested fields of Matrix -> ST -> Matrix). This is probably a rare use-case (we have never nested fields like that in 15+ projects) and would already solve the issue for a majority of users.

darylknight commented 1 year ago

@engram-design this is still happening - please consider updating. I make one change to a content block, and every single content block gets a changedFieldIndicator update.

engram-design commented 1 year ago

I’ll consider looking at this, but honestly the priority is to ensure project config files are updated, rather than not. I know this is quite annoying, but it’s far better for us to ensure that people’s fields are updating and saving correctly.

this is, I believe specific to Matrix > ST > Matrix, where the Super Table field doesn’t get a project config change trigger event from Matrix.

darylknight commented 1 year ago

Thank you. Our use case is only ever Matrix > Super Table. We never put Matrix fields inside Super Table.

engram-design commented 1 year ago

I’ll need to refresh my memory in this one, but could also affect just Mattie > ST. So long as Matrix is the outer field, it had issues triggering project config changes that ST could pick up.

weotch commented 1 year ago

Does the value of changedFieldIndicator matter with respect to resolving git conflicts on this field?

image

neildcuk commented 1 year ago

Would also appreciate understanding the above ☝️

gbowne-quickbase commented 10 months ago

Can we get some resolution on this? Whenever we as much as breathe near a matrix field, we get 200 yaml file changes and devs have to sort through which are actually real and which are fake.

KettyLezama commented 9 months ago

Does the value of changedFieldIndicator matter with respect to resolving git conflicts on this field?

Hi! Can we at the very least have an answer to this question? And has anyone else found a workaround to dealing with this? It significantly slows down our workflow, as other people have mentioned.

engram-design commented 9 months ago

Yes, the value does matter, as it needs to be random to ensure that an update is triggered.

as I’ve mentioned before the priority is ensuring that your fields actually work when modifying them. As annoying and inconvenient as this is, it’d be more annoying if your changes weren’t being saved against fields. It’s a heavy-handed approach for sure, but a limitation of combining Matrix and Super Table.

MoritzLost commented 9 months ago

@engram-design How about only creating a new changedFieldIndicator if any properties of the super table have actually changed? This would cut out 98% of the noise. This is really only a problem because it happens any time the surrounding matrix field is saved, even if you don't touch the super table fields.

engram-design commented 9 months ago

From memory, the parent Matrix field wasn't picking up on changed made to the Super Table field, unless we did something as heavy-handed as a change indicator that - yes - notifies the Matrix field about a "change" every time, but I believe that was the only way to ensure that Super Table (and any nested Matrix fields) actually saved updated settings.

I'll take a look, but messing around with this is low priority, as I'm quite concerned about messing up the current working solution.

JamesNock commented 8 months ago

We've hit this a few times, praying for a fix 🙏

vijayakumar-mayilsamy commented 7 months ago

Hi @engram-design,

Due to this issue, We are manually handling all database updates because whenever we merge commits, numerous conflicts arise due to a recurring issue: Each time a Super Table field is saved, the field generates a new random changedFieldIndicator value in the project configuration, even if there are no actual changes to the field. This leads to excessive noise in Git.

Could you please look into this?

maxfenton commented 7 months ago

I'm also afraid that like engram-design said, changing this will break the functionality of synced changes. Perhaps it could be a setting somewhere that defaults to checked, but where one could turn this off. The interface could have a warning that if changes are not syncing you should check the box.

I stopped worrying about the changes and treat it like the timestamp changing in project.yaml. It can be noisy, but at this point everyone I work with knows in code reviews what they're seeing and not to worry about it.