Skip to content
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

Implement code to establish new pattern for building services for Webview Service #203

Closed
lyonsil opened this issue May 22, 2023 · 3 comments · Fixed by #640
Closed

Implement code to establish new pattern for building services for Webview Service #203

lyonsil opened this issue May 22, 2023 · 3 comments · Fixed by #640
Assignees
Labels
internal Internal issue that doesn't directly affect extension developers or users

Comments

@lyonsil
Copy link
Member

lyonsil commented May 22, 2023

From our discussions on Discord, we agreed:

  • We should eliminate the use of isRenderer(), isExtensionHost(), etc. to fold all the code differences between processes into a single method.
  • Client/server implementations should not be locked into sharing the same interface.
  • We need good names for the client and server parts of a service. One idea was that "XYZServiceClient" could be simply "XYZService", and "XYZServiceServer" could be "XYZServiceProvider."
  • We should have a clear place to put helper code shared between the client and server so it doesn't need to be copied. It's not really an interface, model, or service. Maybe something like would belong in utils, but it probably needs some more thought.
  • We can use main...cross-process-server-model as a starting point, but this example should be streamlined and not used as-is.
@tjcouch-sil tjcouch-sil changed the title Implement code to establish new pattern for buliding services Implement code to establish new pattern for building services May 23, 2023
@tjcouch-sil
Copy link
Member

When we implement this change for the web-view.service.ts, let's move most of the utility code that is in paranext-dock-layout.components.tsx into the renderer version of web-view.service.ts as well as the import of testLayout. Most of that code is only in the component because web-view.service.ts is a shared file and cannot import the various renderer components and such.

@tjcouch-sil tjcouch-sil added the internal Internal issue that doesn't directly affect extension developers or users label Oct 3, 2023
@tjcouch-sil
Copy link
Member

Let's also fix dynamic imports like this when we do that

@lyonsil
Copy link
Member Author

lyonsil commented Oct 30, 2023

To summarize the new pattern:

  1. Determine if the service is essentially a "computation only library" where everything can be done in whatever process is running the code, or if there is a client-server nature to the service. If it is a computation only library, add the service to "shared" and you're done. Otherwise, keep going.
  2. Decide which process (main, extension host, renderer) should own the server side of the service. Encapsulate that logic inside of a network object (or command if a network object won't work for some reason), and register that network object (or command) in a file named [service-name].service-host.ts in the src/[process-name]/services directory. Generally this is used by calling some initialize method from src/[process-name]/process-name].ts.
  3. Extract an interface from that network object (or input/output types for a command) and put it inside of src/shared/services/[service-name].service-model.ts. This is generally where JSDOC comments for the service should reside. Also put the name of the network object (or command) inside of this service-model file. The service-host file will presumably reference the network object (or command) name from this service-model file.
  4. Implement the client side of the service in src/shared/services/[service-name].service.ts. Generally this will just involve creating a network object in an initialize method and creating a small object that implements the model from the service-model file. All functions in the model should generally call initialize and the network object method or command.
  5. As appropriate, expose the service's client object to papi-frontend and papi-backend.

The general motivations of this pattern are:

  • Make code that can only run in one location obvious where it is running (i.e., the service-host code for one process).
  • Make code that lives in shared truly agnostic to what process it runs in. We don't want to have to look at service code to read it all and see if there are isXXX calls to run code that will only run in one process even though the code lives in shared.
  • Minimize duplication of logic and constants.

@katherinejensen00 katherinejensen00 changed the title Implement code to establish new pattern for building services Implement code to establish new pattern for building services for Webview Service Nov 1, 2023
@lyonsil lyonsil self-assigned this Nov 9, 2023
@lyonsil lyonsil linked a pull request Nov 13, 2023 that will close this issue
@tjcouch-sil tjcouch-sil added this to the 2023-12 Q4 Maintenance milestone Nov 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
internal Internal issue that doesn't directly affect extension developers or users
Projects
Status: ✅ Done
Development

Successfully merging a pull request may close this issue.

2 participants