Skip to content

Conversation

@talldan
Copy link
Contributor

@talldan talldan commented Dec 12, 2018

Description

#10890 raised an issue about side effects caused by mutating attributes. Looking through the docs there wasn't anything covering best practices around the handling of attributes, so I've added a bit to the documentation for setAttributes.

Types of changes

Documentation update

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.

@talldan talldan added the [Type] Developer Documentation Documentation for developers label Dec 12, 2018
@talldan talldan self-assigned this Dec 12, 2018
}
```

When calling `setAttributes`, it's important that attributes are not mutated in the process of updating them. This is considered bad practice, the core of Gutenberg has been written with the same philosophies as Redux and state is treated as immutable data. It can also cause bugs. Objects and arrays are passed as references in JavaScript, so mutating them can affect other bindings to those references, for example an attribute's default value.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This makes a lot of sense to document but I think the lingo might be slightly over the head of many WP developers.

It's obviously good to introduce and use higher level terms so folks can learn and expand their skills, but maybe this can be expanded with some extra context, definitions, etc.?

(This could also link into @mkaz's work on a JS tutorial, so we could point someone to some additional background.)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the feedback @chrisvanpatten—I've simplified the text a bit, tried using a simpler vocabulary.

@talldan talldan force-pushed the update/setattributes-documentation branch from 918afba to 6625cfa Compare December 13, 2018 08:33
Copy link
Contributor

@chrisvanpatten chrisvanpatten left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small tweak request (typo and a new link to more reading), then good to merge 👍

@talldan talldan force-pushed the update/setattributes-documentation branch from 662f72c to 712a592 Compare December 19, 2018 04:28
@talldan talldan added this to the 4.8 milestone Dec 19, 2018
@talldan talldan merged commit 5dba988 into master Dec 19, 2018
@talldan talldan deleted the update/setattributes-documentation branch December 19, 2018 05:59
youknowriad pushed a commit that referenced this pull request Jan 9, 2019
…ibutes (#12811)

* Add section to setAttributes documentation about immutability of attributes

* Simplify explanation of immutable attributes

* Fix typo and add link to redux's immutability docs

Co-Authored-By: talldan <[email protected]>
youknowriad pushed a commit that referenced this pull request Jan 9, 2019
…ibutes (#12811)

* Add section to setAttributes documentation about immutability of attributes

* Simplify explanation of immutable attributes

* Fix typo and add link to redux's immutability docs

Co-Authored-By: talldan <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

[Type] Developer Documentation Documentation for developers

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants