Skip to content

Commit

Permalink
Add guidance for monkey patching (#355)
Browse files Browse the repository at this point in the history
Co-authored-by: Anne van Kesteren <[email protected]>
Co-authored-by: Rossen Atanassov <[email protected]>
Co-authored-by: Amy Guy <[email protected]>
Co-authored-by: Daniel Appelquist <[email protected]>
Co-authored-by: Lea Verou <[email protected]>
Co-authored-by: Peter Linss <[email protected]>
  • Loading branch information
7 people authored and hober committed Apr 20, 2023
1 parent 34eae6a commit 8bd2711
Showing 1 changed file with 33 additions and 10 deletions.
43 changes: 33 additions & 10 deletions index.bs
Original file line number Diff line number Diff line change
Expand Up @@ -2670,17 +2670,8 @@ to integrate changes to their manifest format immediately.
This may be due to process (like going to CR),
or due to the addition having a different scope,
like extensions to Web App Manifest only affecting store or payment use-cases.
In that case, it is acceptable to monkey-patch
In that case, it is acceptable to [=monkey patch=]
as long as that is agreed with the original spec editors.

ISSUE(184): when we write up a principle on monkey patching,
be sure to take this nuance into account.

An example of something that was done as a monkey patch
that is scheduled to be integrated into the web app manifest in a future level (post-CR):

* https://wicg.github.io/web-share-target/#extension-to-the-web-app-manifest

</div>

However, if your feature requires a complex set of metadata specific to a functional domain,
Expand Down Expand Up @@ -2865,6 +2856,7 @@ and makes it clear when the state described by the flags is reset.

<!-- TODO: add examples -->

<<<<<<< HEAD
<h3 id="implementability">Resolving tension between interoperability and implementability</h3>

<!--
Expand Down Expand Up @@ -2983,6 +2975,37 @@ You may also find that none of these options is acceptable to your group.
While the best path forward may be to return to user needs to see if the problem can be approached from another angle,
there is the risk that delay may cause some implementations
may ship the feature as a nonstandard API.
=======
<h3 id="monkey-patching">Avoid monkey patching</h3>
A <dfn export>monkey patch</dfn> layers new functionality on top of an existing specification in a way that extends, overrides, or otherwise modifies the existing specification's behavior.
<a href="https://en.wikipedia.org/wiki/Monkey_patch">Monkey patching</a> <strong>is generally considered bad practice and should be avoided</strong> for the reasons listed below; but is sometimes unavoidable
(see our guidance <a href="cant-avoid-monkey-patching">if you need to monkey patch</a>).

An example of monkey patching would be: specification A defines an internal algorithm part of its capability, then specification B overrides or modifies said algorithm with functionality directly, not using publicly defined extension points.

As such, monkey patching (wrongly!) presupposes that underlying functionality cannot, and will not, be changed. This can lead to several problems:

* If the existing specification changes, the monkey patched text may no longer apply thus causing it to break or lead to confusion.
* If several specifications monkey patch part of another spec, the order in which the patches are applied can lead to inconsistent behavior.
* The existing specification authors may not be aware of the monkey patched text in your spec. This misses an opportunity to receive guidance on reaching consensus on a fix. Or worse, the behavior that is being patched is interrelated to so other issue or behavior, making the overall situation worse.
* An implementer doing code maintenance may read underlying specification and inadvertently revert the monkey patched behavior assuming it is a bug.

Wikipedia also describes some <a href="https://en.wikipedia.org/wiki/Monkey_patch#Pitfalls">additional pitfalls of monkey patching</a>.

<h4 id="cant-avoid-monkey-patching">If you need to monkey patch</h4>
Sometimes monkey patching is unavoidable (e.g., early in the design phase of a new specification). If you find yourself needing to monkey patch an existing specification, make sure you:

1. File a bug, or create a fix, for the existing specification. This will help you understand what the problem is and how to fix it, and maybe even alleviate you from even creating the monkey patch in the first place.
1. In your spec, clearly mark the monkey patch as a temporary workaround (e.g., using a big red box) and make it clear to readers that you intend to add your patch to the existing specification. Make sure you link to the existing specification and the bug or fix that you filed.
1. Work together with the authors of the existing specification on a solution. If the existing specification is no longer maintained... Congratulations! you are now the editor of that existing specification. If necessary, add suitable extension points to the existing specification to better handle your changes.
1. Once the existing specification has integrated your monkey patch, remove the monkey patch from your specification.

Note that [=monkey patching=] is different from "modularization", which extends existing technology in a way that is self-contained and doesn't cause side-effects to other underlying specifications (e.g., the CSS Modules).
Modularization is considered a good practice.

It's also usually ok to extend a specification using WebIDL's [=partial interfaces=], [=partial dictionaries=], and so on.
But even in those cases, it's highly advisable to coordinate with the authors of the specification being extended.
>>>>>>> 06e9a85 (Add guidance for monkey patching (#355))

<h2 id="naming-is-hard">Naming principles</h2>

Expand Down

0 comments on commit 8bd2711

Please sign in to comment.