This repository has been archived by the owner on Mar 1, 2024. It is now read-only.
Convert frontend javascript to typescript and refactor. #63
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…ss and make icon images show
… and fixed logger bug for ff and ios
hmuurine
reviewed
Jan 20, 2023
hmuurine
reviewed
Jan 20, 2023
Add missing typescript dependency. Co-authored-by: hmuurine <hmuurine@users.noreply.github.com> Signed-off-by: Luke Bermingham <1215582+lukehb@users.noreply.github.com>
Add missing typescripe and webpack dependencies. Co-authored-by: hmuurine <hmuurine@users.noreply.github.com> Signed-off-by: Luke Bermingham <1215582+lukehb@users.noreply.github.com>
hmuurine
reviewed
Jan 23, 2023
hmuurine
approved these changes
Feb 1, 2023
There was a problem hiding this comment.
Decoupling the default UI from the business logic and providing the functionality as a library might not be in the scope of this PR, so maybe those should be left for the future. The main thing here was Typescript conversion, which looks good to me! 👍
(Maybe could reword the title of this PR before merge?)
lukehb
changed the title
…g other than port 80
…path during frontend install.
lukehb
commented
Feb 6, 2023
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR addresses. #9.
The reference frontend code that ships in this repository is intended to act as a starting point that developers can modify and extend to suit the needs of their Pixel Streaming applications. Unfortunately, the structure of the existing JavaScript code does not enforce a clean separation between the base code and any custom modifications, intermingling everything in a way that makes it difficult to change either the base code or the modifications in isolation. This makes the process of porting customisations from one version of the frontend to another a tedious and labour-intensive process, significantly hindering developers' ability to update their code when new versions of the frontend are released. This is particularly problematic when considering the fact that new releases of web browsers can break the frontend (and this has happened multiple times in the past), so the ability to consume updates as quickly and easily as possible is critical to ensuring that Pixel Streaming applications remain accessible to all users when deployed in a production environment.
To address these limitations, this pull request draws on code from the TypeScript frontend implementation used by Scalable Pixel Streaming and merges it into the upstream reference implementation, with additional changes and improvements to ensure parity with the functionality present in the existing JavaScript implementation. The Scalable Pixel Streaming frontend code was contributed by my colleagues Adam Rehn, Adrian Zahra, Daniel Holden and David MacPherson from TensorWorks.
The new TypeScript library is a modular and extensible package that has been explicitly designed to ensure a clean separation between the base code and any custom modifications. The new TypeScript implementation acts as a drop-in replacement for the existing JavaScript implementation and introduces the following benefits:
Works with modern web development workflows: once the accompanying GitHub Actions CI/CD workflows are in place, the TypeScript library will be provided as both an NPM package through GitHub Packages and a UMD module through this repository's releases, making it easy to consume the library from either TypeScript code or plain JavaScript code using modern web development tools and workflows.
Easy to integrate with frontend web frameworks: all of the core logic in the library can be overridden by extending the base
Applicationclass and providing alternate implementations for the relevant methods. This makes it straightforward to write derived implementations that integrate the library with anything from plain DOM APIs all the way through to complex frontend web frameworks. The library ships with a default implementation that demonstrates the use of the baseApplicationclass with its default behaviour, which can be used as a drop-in replacement for the existing HTML, CSS and JavaScript shipped in this repository.Easy to update: the clean separation between the core logic and custom derived implementations allows developers to rapidly update to new releases of the frontend and stay on top of any WebRTC changes in new web browser updates. Simply update the dependency version where you reference the library (e.g. build configuration files or a
<script>tag) and you're good to go.Supports custom protocol extensions: the TypeScript library makes it possible to extend the WebSocket protocol used to communicate with the Pixel Streaming signalling server, by providing hooks for registering custom message types. This allows downstream projects that use custom signalling servers (e.g. Scalable Pixel Streaming) to consume the reference frontend library whilst still retaining support for their specific protocol extensions.