Improve plugin static typing docs #97
Conversation
|
Deploy preview ready. Built with commit 9a533c3 |
| provides({bar}) { | ||
| return new ServerFoo(); | ||
| } | ||
| }): any): FusionPlugin<ServerFooDeps, ServerFoo>); |
There was a problem hiding this comment.
Are we ok with this as a recommendation?
|
|
||
| It's possible for plugins to be non-trivial enough that they have similar but not identical type signatures on server and browser. | ||
|
|
||
| The recommended way to type those types of plugins is to create separate plugins and tokens for each environment: |
There was a problem hiding this comment.
For what it's worth, we don't follow this advice internally. There are a few places where we use the same types across environments when the actual types differ.
This is (1) partially due to retrofitting types after the fact, and (2) to avoid introducing new environment-specific tokens for the same plugins.
There was a problem hiding this comment.
Yeah, I'm aware that we basically lie for our own plugins, but I think for new development, we should discourage people from following that practice. Thoughts?
There was a problem hiding this comment.
Would this introduce a new implicit dependency between the browser and node plugins? What happens if a user registers one of these tokens but not the other? With a single environment-agnostic token, we don't run into this problem today.
I wonder if this is maybe a good case for why we might want to revisit more helper utilities for creating and exporting plugins.
I could see something like export default exportPlugin(browserPlugin, nodePlugin) or createBrowserPlugin and createNodePlugin to obfuscate any typing magic we do away from the user.
There was a problem hiding this comment.
I think they'd be different problems, but I'm more on board w/ the issues of having two separate tokens for server/browser registrations. In that case, at best, you get an error saying "Such and such token was not registered" and at worst, the runtime simply does nothing because you forgot to add the code to do it.
With a single token overloading both server and client type, there's no way to correctly type either server-side or browser-side usage. At best you need to add type checks everywhere to disambiguate a A | B type, and at worst you get uncaught runtime type errors because you used a part of the server type signature in the browser or vice-versa.
There was a problem hiding this comment.
Agreed that from a static typing perspective, different tokens makes our lives easier as it avoids the need for any hacks or type refinements.
I don't think we have (or feasibly can) address the issue of the client and server implementations having an implicit dependency upon each other that isn't codified anywhere. We do not have any error handling around this if done today to say "you are missing the server implementation for this plugin" for example. It would be the worst case from above - runtime does not work as expected, since only half of the plugin is working. A cursory glance may confuse a user as it looks like the code is there.
That being said, this trade-off is possibly sufficient if we are looking to avoid any compiler hacks (e.g. utility functions to hide this) and want a cleaner static typing experience. I'd be interested to see what other folks' opinions are here, and whether we should adopt this approach for our plugins as well.
There was a problem hiding this comment.
Also just a little concerned that we would give this advice if we don't follow it ourselves. If we are going to document this approach, we should probably update our existing plugins to as well. It might be worth making a list of migrations we would have to do to follow this?
No description provided.