Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(mock-doc): make MockAttributeMap iterable #2788

Merged
merged 3 commits into from
Jan 7, 2021

Conversation

simonhaenisch
Copy link
Contributor

@simonhaenisch simonhaenisch commented Jan 5, 2021

TL;DR Proposed changes:

  • implement [Symbol.iterator] in MockAttributeMap to make it iterable
  • implement get [Symbol.toStringTag] in MockAttributeMap
  • don't pass symbols into isNaN in attribute proxy
  • prettier formatted a couple things

When writing E2E tests I ran into different PrettyFormatPluginError errors, when Jest would try to pretty-format an E2EElement while iterating over its attributes:

PrettyFormatPluginError: Cannot read property 'name' of undefinedTypeError: Cannot read property 'name' of undefined

or

PrettyFormatPluginError: _val$hasAttribute.call is not a functionTypeError: _val$hasAttribute.call is not a function
at node_modules/jest-matcher-utils/node_modules/pretty-format/build/plugins/DOMElement.js

This error might only happen in my case because I'm using jest@26 and I think Stencil still defaults to jest@25, but doesn't complain if a higher major version is used cause it's likely compatible.

Anyway, after debugging this for half a day, I figured out that pretty-format fails to print the attributes because MockAttributeMap is not really iterable but it tries to iterate it (passed into Array.from, see facebook/jest/pretty-format/src/plugins/DOMElement.ts#L87 and facebook/jest/pretty-format/src/plugins/DOMElement.ts#L87), and ends up being an Array of undefineds (not sure why).

The solution is to add an implementation of [Symbol.iterator]. Another symbol that pretty-format tries to access is Symbol.toStringTag so I've also implemented that.


Also, the attribute proxy uses isNaN which breaks when called on a symbol:

PrettyFormatPluginError: Cannot convert a Symbol value to a numberTypeError: Cannot convert a Symbol value to a number
        at isNaN (<anonymous>)

      at Object.get (node_modules/@stencil/core/mock-doc/index.cjs:21:10)
          at Proxy.toString (<anonymous>)
          at Array.map (<anonymous>)

thus I've added a check for that.

@simonhaenisch simonhaenisch marked this pull request as ready for review January 5, 2021 20:29
const attr = new MockAttr('attr', 'value');
map.setNamedItem(attr);

expect(Array.from(map)[0]).toBe(attr);
Copy link
Contributor Author

@simonhaenisch simonhaenisch Jan 5, 2021

Choose a reason for hiding this comment

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

I've confirmed that this test works by removing my [Symbol.iterator] implementation. Array.from(map) would then yield [undefined] instead of [MockAttr].

@adamdbradley adamdbradley merged commit 1aa9cae into master Jan 7, 2021
@adamdbradley adamdbradley deleted the fix-mock-attribute-map branch January 7, 2021 16:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants