-
-
Notifications
You must be signed in to change notification settings - Fork 3k
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
chore: upgrade dependencies #583
Conversation
Interesting, the preactjs/compressed-size-action still fails with the same error during the build step but the node 12 ubuntu-latest test successfully installs and fails at the Still needs work, but I think it's closer. @tannerlinsley where do we go from here? |
Let’s delete the size workflow. Useless tbh |
I'm glad you said that because I didn't understand it either haha. But do you have any idea how to fix the suspense hook? I wasn't able to reproduce the problem in a codesandbox so my best guess is that it's a issue with one of the test libraries and possibly re-rendering twice before the The best I was able to do was:
But this seems like a really bad solution as it's no longer a representative test of reality. |
Yeah, there’s something tricky going on for sure. I think that’s fine for now. We know it works in the wild
…On Jun 14, 2020, 3:39 PM -0600, CreativeTechGuy ***@***.***>, wrote:
I'm glad you said that because I didn't understand it either haha. But do you have any idea how to fix the suspense hook? I wasn't able to reproduce the problem in a codesandbox so my best guess is that it's a issue with one of the test libraries and possibly re-rendering twice before the setTimeout is able to fire.
The best I was able to do was:
fireEvent.click(rendered.getByText('retry'))
await sleep(100)
fireEvent.click(rendered.getByText('retry'))
But this seems like a really bad solution as it's no longer a representative test of reality.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
I don't think we should leave tests failing that we expect to fail. That's a really bad impression to give off. I'd vote to mark the suspense features in the docs as experimental (because suspense for data fetching itself is experimental) and disable (or rewrite) the tests for now. If someone wants to use those features it's buyer beware. The concern with this approach is that there may be people using the feature in production and losing confidence by removing tests isn't good. |
Let’s just do what we need to do to make it pass for now
…On Jun 14, 2020, 3:47 PM -0600, CreativeTechGuy ***@***.***>, wrote:
I don't think we should leave tests failing that we expect to fail. That's a really bad impression to give off.
I'd vote to mark the suspense features in the docs as experimental (because suspense for data fetching itself is experimental) and disable (or rewrite) the tests for now. If someone wants to use those features it's buyer beware. The concern with this approach is that there may be people using the feature in production and losing confidence by removing tests isn't good.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
Removed workflow, added workaround for test with explanation. The CI tests now succeed! |
This is great! Would you mind applying this to the next branch too? |
What is the |
🎉 This PR is included in version 1.5.8 🎉 The release is available on: Your semantic-release bot 📦🚀 |
It’s the next major version, but under construction.
…On Jun 14, 2020, 7:16 PM -0600, CreativeTechGuy ***@***.***>, wrote:
What is the next branch? I couldn't find the process for that documented of what goes there.
—
You are receiving this because you modified the open/close state.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
For the past several weeks CI has been failing at the installation stage with the error:
In an attempt to fix this, I opted to update the dependencies to see if that solves the issue. All dependencies are updated to the latest versions. This mostly affects the tests which needed to be updated as it now used deprecated features. I diffed the output bundles before and after the change and there was no meaningful difference.
Note, there is one failing
useQuery
test (should reset error state if new component instances are mounted
) which I presume is failing due to a timing issue with this setTimeout. I was not able to fix this test but at least now we should be able to see accurate CI results that it's failing and can follow up with fixing the test or the library. This test error was introduced with this upgrade, but only in the unit tests as the output bundle has no change.