-
-
Notifications
You must be signed in to change notification settings - Fork 16
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
Hydration fails when a component is used on multiple pages #96
Comments
I’ll add that it’s entirely possible something strange in my setup is causing this, but I’ve tried to eliminate that possibility (and to find workarounds) before reporting. But so far both efforts have come up empty. |
Hmm, I'll have to look into this. You've definitely diagnosed the problem correctly, I don't think it's anything with your setup. I'm trying to remember if there was a reason I was ignoring the shared chunk in As for the tests, yes please! 🎉 Microsite desperately needs tests. I was moving fast and breaking things (and also didn't expect this gain traction) so I skipped them. I think |
If your thinking is anything like mine, I wouldn’t be surprised if the “eventually” part of the comment became an implicit TODO and the naive solution got sidetracked haha. FWIW I did seem to notice other “shared” stuff that wasn’t hydration related getting picked up, though that is definitely something that might be more related to my setup because I’m fairly certain it was some junk from my Fela wrapper library. But it’s possible that shared chunk is a potential bloat. If you haven’t knocked it out before me I’ll look into this more tomorrow with fresh eyes and 🥳 tests!
Oh I hadn’t heard of this one. I’m so so glad you didn’t say Jest (though that’s the only one I know really well). I’ll give that a look tomorrow too. FWIW I’m very surprised to see Ava benchmark so poorly in their writeup, in my (admittedly exploratory) experience it’s been nothing but snappy. This is definitely gonna get me thinking about my own testing preferences, although I’m torn between their respective APIs and feature sets. I have a few more test related questions some of which I’m going to copy/adapt from another project I offered to bring tests which I’m ashamed to admit I dropped the ball ondue to major burnout and a resulting career change, which is a big part of why I’m building a personal site now, link for accountability. I do want to come back to that project when I’ve got other stuff in order but right now I’m not even doing anything that uses a database. but anyway...:
|
Oops I meant to add more on the refactor topic. There are definitely cases where I see opportunities here. If I were to try anything big I’d definitely want to open a discussion issue/chat first. Most prominent are (as mentioned) anywhere I see |
I should also add, part of the reason for all the big upfront questions (it’s okay if you’re squishy on any of them btw) is because I want to make sure I’m being a good citizen/visitor when I contribute, especially if I’m introducing anything new. And also because I see a lot of potential here and want to take it as seriously as I would take starting on a new team with a project I’m gonna be living with and wake up thinking about (which no joke I did today, with my dev setup so close I can almost taste it). Just saying, I’m sorry again for blowing up your notifications but I’m seriously excited to work with this project. |
Thanks so much for the enthusiasm—I'm looking forward to having some help on this project! I'm pretty open to whatever your preferences are on testing, honestly.
I don't have any experience in this area, but I'm not sure how useful type-level testing would be since the codebase is already written in TS. We can consider this later if the need surfaces.
GitHub Actions would be great—happy to lend a hand here.
Nope! I try to focus on productivity more than nitpicking code style. You already seem familiar with the code base so I'm not concerned about it.
The main reason I hadn't tackled tests up until now is because I had no idea how to approach integration or snapshot tests. It's not something I do much of, but I know those would be the most useful. Tbh anything is going to be better than my current (very manual) process.
Eh, small commits are easier to grok but I realize that some tasks (like the initial testing setup) will involve a lot of changes, so I don't have a problem if there are some big ones.
Separate directory within each module works for me
I like BDD!
Totally open to refactors! I'd love the codebase to be as easy to approach as possible. |
@eyelidlessness I invited you to this repo. Feel free to make branches and PRs here instead of on a fork, please just add me as a reviewer on any PRs! |
Wow thank you! Gonna start digging into tests for this issue! |
Following up for EOD because it’s puppy time. I’ve had a heck of a day trying to get testing setup. It’s actually not that bad for single test runs in Uvu (and I even have that working with VSCode debugging), I just made a series of dumb mistakes like not stopping to ask for directions (I missed that Uvu requires an explicit call to run) and not taking breaks. But that’s all working and if I can get a few minutes of focus at the keyboard tonight I’ll push it up in a branch, otherwise first step tomorrow. The other place my day went off the rails is a fairly typical one for me. Uvu doesn’t have a watch mode, which is pretty much my entire dev flow: start up test runner in watch/debug mode, red-green-refactor. So I did the stupid things I’d normally do:
I’m reporting failure on that front. These tools don’t like each other. And I’ve learned yet more about the hell that ESM/CJS interop can be. However! Once I got the nerve to take a break I also stopped to ask for more directions. And I think there’s a good chance Estrella is a potential solution here. It’s not its primary use case but it has watch mode built in, it’s using esbuild, it’s designed for customization, and it has a nifty CLI-first interface. So I’ll be looking at this tomorrow (absent objections). I was really hoping to get further on this front today, but honestly every day since I’ve returned to frontend dev has felt like that. While I’m here, I know the license allows it but I’d like to ask explicit permission to push out next builds (namespaced to me on NPM so it’s clear they’re not official cuts unless you’d prefer otherwise) so I can also evaluate them on my own project without doing the horrific npm/yarn link dance. Assuming I even get so far as that. But I’m simultaneously tracking contribution here as well as my own site and I’m so close to being able to build my site I can taste it, and trying to do everything I can to get out of my own way as possible on that. |
@eyelidlessness Awesome, thanks for the update! I'm not necessarily tied to Feel free to put namespaced builds out if you'd like! The |
Great news! I got a little proof of concept with Estrella working, and goodness it is fast! And it took very little fuss to get going. I'm going to clean up some mess, setup a GitHub Action, and push up a PR. It's not a huge diff, but I think I'd like to keep test setup separate from fixing this issue to get a meaningful win. Hopefully ready within the hour! |
@eyelidlessness I remembered the reasoning here—I had assumed that components in the |
It’s entirely possible I was misunderstanding something about intended usage! I was just validating that I could hydrate a dumb counter component, and include it on multiple pages. Is there some additional ceremony required for that? Or is that what you mean by “that assumption doesn’t hold”? For real-world use I definitely intend to share an interactive component across pages, for instance a dark mode toggle. |
Without getting too hung up on the big testing question, I strongly suspect the resolution of this is pretty simple:
|
I can confirm that these proposed changes do include my expected hydration behavior, and that moving/excluding shared static content does prevent the shared chunk bloat I saw before. I need to run some errands but I’ll have a small PR for this hopefully this afternoon/evening. |
Oh I wanted to mention, I originally tried branching off #104 to make these changes, but builds in my project failed due to the removed |
Version: 1.1.0-next.3
Steps to reproduce
Expected behavior
The component will hydrate on each page.
Observed behavior
No JS is loaded at all.
Apparent cause
Any shared component with hydration is bundled as a shared chunk, but the shared chunk is excluded from
hydrateBindings
, and doesn’t appear to be referenced anywhere else. If I comment out that check it does hydrate as expected.I was tempted to offer a PR removing that check, but I can’t shake the feeling that there’s a reason it’s there and the intent was to handle that case differently.
Related side note: in terms of contribution, my usual approach is to write a failing test exercising my bug, then fix to make the test pass. But at present Microsite doesn’t appear to have a test suite. Is that something you’d be interested in? If so do you have a preferred test framework? I’d be happy to look into setting up CI for any tests I contribute, and to try to increase coverage as I contribute.
The text was updated successfully, but these errors were encountered: