Closed stevensacks closed 3 years ago
I found the Chromatic delay documentation.
I added this to my preview.js parameters
chromatic: {delay: 300},
And that solved the issue.
It would probably be good to add a warning about needing to add a delay for Chromatic when using the msw addon.
Hey, @stevensacks. Thank you for reporting this.
While setting the chromatic.delay
option resolves the issue, it's rather a workaround than a solution.
The msw addon captures all http requests, not just the ones you assign it. If there isn't a defined handler, it passes it through.
Yes, this is correct and expected.
I am using google fonts and MSW tries to load the google fonts itself before passing the request through to google
I'm not sure what you mean by that. This is what's going to happen when an unhandled request happens:
response:*
hooks. Is it this step that causes the delay for you?Note that the worker script is in your control and you can try disabling certain parts of its workflow (useful for debugging only). For instance, is the issue still there if you comment the part that reads the response body?
MSW captures all requests and when this happens, even if it is unhandled, some latency is introduced.
The amount of latency in loading google fonts is inconsistent. I don't have a way to measure it since it is happening during Chromatic's build+snapshot process. The latency can be significant enough that it causes Chromatic to render a story before the google fonts load, or it can be fast enough that the fonts load in time. The latency changes from build to build, story to story, so on one build for a particular story they load in time, and another build they don't. When this happens, Chromatic reports it as a design change to be reviewed and it's not readily apparent why, especially if the fonts are similar.
This is just a consequence of using MSW with Chromatic. I'm not sure that there's anything that can be done about it within MSW or if it's even worth doing since Chromatic has a delay parameter to solve this exact type of problem, be it with MSW or anything else that has inconsistent loading times.
Despite using Chromatic for over a year, I didn't know it had a delay parameter when I first reported this issue here. I'm sure that I am not alone in that I didn't read the entire documentation for Chromatic when I started using it. After I reported the issue here, I googled and found the Chromatic documentation about its delay parameter, implemented it, and that resolved the issue.
What I'm suggesting is adding a heads up in the MSW Storybook addon README to save future developers the headache of running into this issue unawares. A brief mention that the MSW Storybook plugin adds latency (by its very nature), and if you are using Chromatic, here's a link to the documentation for the delay parameter in case you need to add some to give MSW time to finish responding to requests before Chromatic takes the snapshot.
Any network requests are subjected to latency, regardless if they're processed by a Service Worker. The fact that the Service Worker introduces latency in your case doesn't create an issue, only allows it to surface. There's a race condition between your request to Google Fonts and your test, that's why you get an intermittent test behavior.
Using timeouts is not a good idea to solve race conditions. Instead, consider awaiting the response from Google Fonts before running your tests. I usually do something like .goto('/url', { waitUntil: 'networkidle' })
with Playwright. There should be a Chromatic alternative for awaiting the network as well.
There is no add-on-specific issue here to add any notices in the README. Race conditions must be synchronized in tests to prevent flaky behavior.
Before I added msw, google fonts loaded in time for Chromatic snapshots 100% of the time.
After I added msw, the latency issue with loading google fonts appeared.
You wrote a plugin for Storybook (and therefore Chromatic). In so doing, you have a responsibility to acknowledge that this plugin "surfaces an issue" (as you put it) with Chromatic so that people who use your plugin know how to resolve it. If you have complaints about how Storybook works and how Chromatic solves what you label as a "race condition", then you should bring them up with the Storybook team. You shouldn't put the burden on the people who use this plugin.
Is it really too much to ask to give a helping hand to people who use this plugin by letting them know in a simple sentence in the README about the issue that this plugin "surfaces" and the solution that Chromatic provides to resolve it?
People who use Storybook and Chromatic don't have control over its inner workings any more than you do. I'm not blaming you for the issue that this plugin "surfaces". It's not a shortcoming in your code. But since we don't have control over the system in which this plugin runs, we should at least acknowledge that the problem that this plugin "surfaces" has a solution (imperfect as you might find it to be) until such a time that the underlying issue can be resolved by the Chromatic team.
Sorry for phrasing it wrong, perhaps. My statement above explains that the issue you experience isn't related to the add-on. Network latency may occur at any time and must be accounted for in your setup. If Service Worker introduces a latency, there's little we can do, as we haven't authored the Service Worker spec. Regardless, it's not an issue to be addressed on the add-on's side. Perhaps raising this with Chromatic is the right direction, and I strongly encourage you to do that.
As an open-source maintainer, I dedicate an insane amount of time to creating and supporting dozens of projects in my free time. As much as I want to help each and every user, please don't expect me to solve everything for you. I strongly dislike your tone, and this isn't the first issue you're expressing yourself as if this project owns you something. It doesn't. I don't collaborate with Chromatic at the moment, and you can raise this very issue with them much faster than I can, given you have the foremost interest in having this solved. Not to say that everything here is open-sourced, and you can investigate the issue and provide a fix instead of expecting the maintainers to do so.
I don't see a valid reason to mention anything in our documentation, as there's no add-on-specific issue our users should know. If we follow this suggestion, we may as well add dozens of other general rules regarding relying on asynchronous code, which has nothing to do with this plugin or the underlying libraries it uses.
I don't deny the possibility that there's an issue somewhere. But until that is proven, I treat this as a general race condition issue in your setup, as we don't have anything specific that'd affect the latency.
I think there's a miscommunication here, and you perceiving a negative tone is due to a limitation of text as a medium, and this being made into a bigger deal than it is.
I would like to clarify a few things.
First and foremost, I respect you as a developer and I am grateful that both msw and this Storybook plugin for msw exist. Second, I am not saying that there is anything wrong with this Storybook plugin or msw. And lastly, I am not asking you to solve anything.
All I am asking for is a sentence and link in the README file to save time for developers who are using this plugin in combination with Chromatic (which is an extension of Storybook) to be aware of the workaround for this issue, an issue which is known to the Chromatic team and one for which they have created and documented a workaround for.
It seemed odd to me that you were pushing back so hard on something so simple to add that would help everyone who used this Storybook plugin in combination with Chromatic. If I'm reading you correctly, it seems you're saying you're not convinced that the issue is with the combination of Chromatic and this plugin, but something to do with my particular Storybook setup, and that is why you aren't willing to add a note to the README.
Prior to adding the msw storybook plugin, the google fonts always loaded in time. I've had my Storybook+Chromatic set up on this project for almost a year and the google fonts were in place from the first day. In all that time, there was never once a problem.
Last month, I created a new branch, added the msw plugin and pushed to confirm that my Storybook still worked and built correctly on Chromatic before I modified any of my stories. In that build, Chromatic reported numerous component stories needed to be reviewed because they had changed. It took some time to figure out that it was because the google fonts weren't loading on some of the stories. When I looked at the Storybook on Chromatic, it seemed fine. I didn't connect that it was msw at the time. I thought maybe it was a transient issue, so I left it alone, assuming the next build it might correct itself.
I added a simple component, and on the next Chromatic build some of the components that it said had changed and needed to be reviewed now no longer needed to be reviewed (i.e. they were again the same as the build before I added msw), some of them still said they needed to be reviewed, and now other components needed to be reviewed.
I looked in my console when running Storybook and saw that msw was outputting that it was passing through the unhandled calls to google fonts. I wondered if that could be it. I removed the msw Storybook plugin from the configuration (since I wasn't using it yet) and on the next build Chromatic had zero stories to be reviewed. Google fonts were loading as before and the stories no longer had any differences.
That's when I posted this issue. It was clear to me that msw introduced latency, however slight, and was causing google fonts to inconsistently finish loading before Chromatic took its snapshots.
Then, I did some googling and found the delay parameter in Chromatic, at which point I came back here and shared that information and suggested adding a note about it to the README to save the next developer some time.
That's all there is to it. I was just trying to help let people know about an integration issue between Storybook, Chromatic, and the msw plugin. It makes sense that the latency loading google fonts that was introduced by msw would cause an issue with Chromatic snapshot timing. The Chromatic team says as much in their documentation for the delay parameter.
I don't think the perfect solution is easy for the Chromatic team to implement, and so they have an imperfect, but pragmatic solution for this exact type of issue. I'm asking to extend that pragmatism by mentioning in the README for this plugin that Chromatic has a workaround for this issue, an issue I didn't even know existed nor that there was a solution for it because the msw storybook plugin was the first thing I ever added to my storybook setup that introduced latency when rendering stories.
Could other developers go through the same process I did and learn the hard way? Sure. But isn't one of the great things about open source sharing of knowledge? I could write a blog article about this instead, and when people inevitably search to figure out what's happening when they run into this issue, they might find my article. Wouldn't it be easier if there was a single sentence in the README of this plugin that would save them all those steps?
I've discovered a conflict with the msw addon and Chromatic that needs to be addressed.
The msw addon captures all http requests, not just the ones you assign it. If there isn't a defined handler, it passes it through.
I am using google fonts and MSW tries to load the google fonts itself before passing the request through to google (I can see it in the console warning locally).
There's a delay in this pass-through that causes Chromatic snapshots to be taken before or after the google font loads in every story. The timing is inconsistent.
I am using a google font via the preview-head.html.
Because of this, I have to resolve dozens of differences every time I commit because the fonts are rendering differently each time in every story. Sometimes the google font loads in time, sometimes it doesn't. When it doesn't, it's using the fallback font. It's inconsistent so different stories are different each time.