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

Add test for Preact support #75

Merged
merged 11 commits into from
May 8, 2020
Merged

Conversation

kale-stew
Copy link
Contributor

@kale-stew kale-stew commented May 6, 2020

Description

This PR introduces tests to the Preact support that was introduced in #67.

We still need to figure out how to pass props directly to the updated Preact component being updated here to verify the container's update fn is invoked (test case no. 3)

Checklist:

  • All tests are passing (Not yet!)
  • Benchmark performance has not significantly decreased
  • Bundle size has not been significantly impacted
  • The bundle size badge has been updated to reflect the new size

Bundle size is not impacted by tests

test/node/advanced.spec.js Outdated Show resolved Hide resolved
@kale-stew
Copy link
Contributor Author

kale-stew commented May 8, 2020

Ok, replied to the above feedback and got the tests synced up with each other, but now we're seeing legitimate failures for actual test conditions.

Case 1 - compares without warning or errors: expected 0, got 1
Case 2 - elements of same type and props are equal: expected 1, got 2

Edit: It looks like our Preact check isn't passing the Preact elements we've created in these tests whatsoever, which is why it's always re-rendering. Going to evaluate the Preact elements themselves, I assume I've duplicated React behavior that is incompatible with PreactChild and PreactContainer.

Edit, part 2: The offending check is a.$$typeOf which only returns React element data, it's coming back as undefined for everything Preact. Going to tie it to the React validator for now, until I can get more feedback from more Preact-smart folks 👍

@kale-stew kale-stew force-pushed the tests/preact-support branch 2 times, most recently from 9551571 to 5570ead Compare May 8, 2020 05:43
@kale-stew
Copy link
Contributor Author

Our tests are now failing because @testing-library/dom is incompatible with the node v8 engine that AppVeyor runs first. 😞

The library only just recently patch released support for Node v10 (one whole version newer) 13 days ago and has stated they plan to deprecate support whenever it's officially EOL (sometime mid-2021). This will be an issue for our current CI approach.

Can we skip specific tests for a specific Node version? Should we go about these tests a different way entirely? What % of users are using preact and node v<10?

Copy link

@JoviDeCroock JoviDeCroock left a comment

Choose a reason for hiding this comment

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

Awesome work 💯

@kale-stew kale-stew force-pushed the tests/preact-support branch 2 times, most recently from a2f9912 to e92a7ea Compare May 8, 2020 17:27
@ryan-roemer
Copy link
Member

@kale-stew -- I've added two small changes:

  1. 11a60b4 - Our coverage wasn't being generated in Node.js.
  2. d7f3b6f - Transpile a specific preact library. You can use this technique for other deps if we find them (if this isn't enough for ie11)

@ryan-roemer
Copy link
Member

Updated cov to codecov (removed coveralls) with badge!

Also note, we're not running travis, but should pass because this branch doesn't target master. Will catch up in preact-support branch.

@@ -1,5 +1,9 @@
# Changelog

## Unreleased

- [#75](https://github.com/FormidableLabs/react-fast-compare/pull/75). Drop support for Node 8.
Copy link
Member

Choose a reason for hiding this comment

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

I'd leave this as "drop test support for Node 8". As our library works untranspiled in ie11, it definitely works in lower nodes 😉

@kale-stew kale-stew merged commit 3938f76 into preact-support May 8, 2020
@kale-stew kale-stew deleted the tests/preact-support branch May 8, 2020 22:11
@kale-stew kale-stew mentioned this pull request May 8, 2020
4 tasks
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.

3 participants