finos / openfin-react-hooks

A collection of React Hooks built on top of the Openfin API - from Scott Logic
Apache License 2.0
48 stars 19 forks source link

Add support for v1 child window (alternative) #57

Closed ms14981 closed 4 years ago

ms14981 commented 4 years ago

Alternative to #52 (where a user must choose the version of the hook to use).

This PR (work in progress) shows how the version of the API the hook uses could be switched at runtime, based off the version found in fin.System.

The main drawback of this is that both v1 and v2 of the code must be in the same hook, because by the rules of hooks, hooks must be at the top level and cannot be within a conditional.

Another alternative would be to expose all the hooks not in their raw form, but as functions that return a hook. This is unlikely to avoid breaking the rules of hooks because the function returning the version (fin.System.getVersion) is async. There might be a hack to read the app.json or app.dev.json and get the property synchronously, but I'm not sure that would be a good idea (even if it works), especially as the signature of the hooks would have to change.

ms14981 commented 4 years ago

This probably could be made nicer by creating an interface to abstract away the version specific code, and having a concrete class for each version.

ms14981 commented 4 years ago

Will merge #59 instead