testing-library / dom-testing-library

🐙 Simple and complete DOM testing utilities that encourage good testing practices.
https://testing-library.com/dom
MIT License
3.25k stars 460 forks source link

Disable callToJSON option used by pretty-format #1315

Open AdrienClairembault opened 1 month ago

AdrienClairembault commented 1 month ago

What:

This PR disable the callToJSON option used by the pretty-format dependency.

Why:

These changes are needed to fix a conflict that happens when trying to add E2E tests to a project that uses Symfony's live components.

The issue is caused by the process of building the "pretty printed" DOM debug data. This process is handled by the pretty-format dependency, which will by default call a toJSON method on any object that defines it.

See the relevant from pretty-format:

image

This seems harmless most of the time but conflict with symfony's live component because the component DOM node is a proxied object which support any method being called on it (which will be interpreted as an "action").

For more context, here is the relevant code used by the proxy:

image

With this in mind, the typeof val.toJSON === 'function' check of pretty-format will be validated and it will call toJSON on symfony DOM items, which will trigger a backend request for a toJSON action, which does not exist, thus breaking all tests.

How:

Luckily pretty-format has a option which can be used to disable the whole toJSON logic.

The testing library use only uses pretty-format to display DOM items which will never support this special method unless they are weirdly proxied. Disabling the option should thus be fairly safe and have no negative impact (the test suite seems to confirm it as all tests passes without issues).

Checklist:

codesandbox-ci[bot] commented 1 month ago

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 59b21a5775f55a63e6439634f14f3b062cf7e02d:

Sandbox Source
react-testing-library-examples Configuration
MatanBobi commented 1 month ago

Thanks @AdrienClairembault. Can you please open an issue with a reproduction of the problem so we'll understand what we're trying to fix here with a real life example? Also, I'm not entirely sure we'd want to merge this change as this looks like the default behavior that fits most use cases.

AdrienClairembault commented 1 month ago

I have opened an issue which hopefully should be a bit clearer.

Also, I'm not entirely sure we'd want to merge this change as this looks like the default behavior that fits most use cases.

I completely understand this and very much respect that you would not blindly change a configuration option and risk potential side effects.

My point of view is that the change would be safe as the option is not relevant when doing DOM nodes comparison but javascript is not my main field at all and I would appreciate if someone with more experience on the subject could confirm it.

AdrienClairembault commented 1 month ago

Also, I am very much aware that this is quite a "niche" issue as the overlap of symfony live components + dom-testing-library users is probably extremely small (guess I am the first one to try this "stack" since no one reported this issue yet).

As a maintainer of others projects I very much understand this kind of changes are often not worth validating on your side as the risk can be high for a single potential user benefiting from it.

I don't mind keeping a patch on me side that works for me and would take no offence if you want to close this for now (at least if someone else has the same issue in the future they might find this thread and be able to keep going using the solution provided here).