Platform scripts audit #567

mcottontensor · 2025-04-03T23:58:40Z

Relevant components:

Problem statement:

The platform scripts have been problematic for a while, causing multiple builds of the same package, slow startup and failed startups.

Solution

Have gone over the windows and linux scripts and made sure they are focused on what needs to be built and only building that. The usage text has also been updated to properly reflect the options.

changeset-bot · 2025-04-03T23:58:45Z

⚠️ No Changeset found

Latest commit: 1bb21da

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

SignallingWebServer/platform_scripts/cmd/common.bat

lukehb

~~I am going to give this a local test on my workspace, I'll report back.~~ It works well! I have some left some feedback below on a fix things to consider.

lukehb · 2025-04-04T06:50:42Z

As an aside, I found when testing this that webpack building umd reference frontend it will selectively use either the /dist/cjs or dist/esm output of each of the packages depending which one exists on disk, with a preference for esm.

In terms of the final bundle that is produced in these two outcomes (deps being either all cjs or all esm) the bundle size is very similar.

Building the dev bundle is about 2.4mb on disk vs prod which is 370kb. (As an additional note when I reviewed the GH releases our bundle size does seem to have grown by about 100kb since October last year for no reason? - Maybe we should use: https://www.npmjs.com/package/webpack-bundle-analyzer )

The reason I mention the bundle size is it fine for start.bat to use larger dev bundle imo, but the GH release ships should prod bundle (which it does not, but just mentioning this in case you make more changes to this PR).

mcottontensor · 2025-04-07T23:06:36Z

The reason I mention the bundle size is it fine for start.bat to use larger dev bundle imo, but the GH release ships should prod bundle (which it does not, but just mentioning this in case you make more changes to this PR).

Looking at the release action it uses the build npm script which calls webpack with the prod configuration?

lukehb · 2025-04-08T01:01:34Z

The reason I mention the bundle size is it fine for start.bat to use larger dev bundle imo, but the GH release ships should prod bundle (which it does not, but just mentioning this in case you make more changes to this PR).

Looking at the release action it uses the build npm script which calls webpack with the prod configuration?

Quite right! Just ignore that bit then. I still think the growth in bundle size is interesting and should be looked into in a new ticket/PR. But is certainly not a blocker for this one.

lukehb

Some rfc

lukehb · 2025-04-04T05:21:36Z

Frontend/implementations/typescript/package.json

        "build": "npx webpack --config webpack.prod.js",
        "build:dev": "npx webpack --config webpack.dev.js",
        "build:esm": "npx webpack --config webpack.esmodule.js",
+        "build:cjs": "npx webpack --config webpack.common.js",


This a funny one actually on closer inspection.

We have the following webpack build configs for the reference frontend:

webpack.common.js - This is the base config used by both webpack.dev.js and webpack.prod.js - but perhaps confusingly named is not actually configuring webpack to build commonjs, but rather umd (which is correct considering you can't run cjs in the browser and webpack transforms the cjs deps into browser compatible umd for us). One solution to avoid this confusion is to rename this file webpack.base.js. Of additional note, because the webpack.common.js configuration does not specify a mode (development or production) webpack emits some warnings saying falling back to development etc, so this config should probably never be called directly.

webpack.dev.js - The dev configuration of the umd build (with inline source maps). This is probably the one we want the start.bat script to use.

webpack.prod.js - The prod configuration of the umd build that does minification etc so you end up with a smaller bundle.

webpack.esmodule.js - A completely different configuration that bundles into an esm module. Which could also be interesting for some usecases (like if someone is an all ESM codebase), but probably umd is the one we and most users will want to use.

Suggested change

"build:cjs": "npx webpack --config webpack.common.js",

"build:prod": "npx webpack --config webpack.prod.js",

lukehb · 2025-04-04T05:23:14Z

SignallingWebServer/platform_scripts/bash/common.sh

 		pushd "${SCRIPT_DIR}/../../../Frontend/implementations/typescript" > /dev/null
-		"${SCRIPT_DIR}/node/bin/npm" install
-		"${SCRIPT_DIR}/node/bin/npm" run build
+		"${NPM}" run build:cjs


Suggested change

"${NPM}" run build:cjs

"${NPM}" run build:dev

lukehb · 2025-04-04T05:23:48Z

SignallingWebServer/platform_scripts/cmd/common.bat

    pushd %CD%\Frontend\implementations\typescript
-    call "%SCRIPT_DIR%node\npm" install
-    call "%SCRIPT_DIR%node\npm" run build
+    call %NPM% run build:cjs


Suggested change

call %NPM% run build:cjs

call %NPM% run build:dev

* Limited the bash script to build cjs * Added a cjs build target * Updating bash script to be a little more friendly with builds. * Added rebuild options to bash script * Updating windows batch script to be inline with bash script. * Cleaned up arguments further for the bash script * Tweaking build behaviour. Updating args * Sorting out webpack configs (cherry picked from commit f7abd33)

github-actions · 2025-04-08T05:07:47Z

💚 All backports created successfully

Status	Branch	Result
✅	UE5.5

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation and see the Github Action logs for details

* Limited the bash script to build cjs * Added a cjs build target * Updating bash script to be a little more friendly with builds. * Added rebuild options to bash script * Updating windows batch script to be inline with bash script. * Cleaned up arguments further for the bash script * Tweaking build behaviour. Updating args * Sorting out webpack configs (cherry picked from commit f7abd33) Co-authored-by: mcottontensor <80377552+mcottontensor@users.noreply.github.com>

* Action audit (#565) (#566) - Removing duplicated push and pull_request triggers - Removing multiple install commands when one root will work. - Removing references to UE5.5 in favor of wildcard matching (cherry picked from commit 4ee5695) Co-authored-by: mcottontensor <80377552+mcottontensor@users.noreply.github.com> * [UE5.5] Moved frontend library from a peer dep to a normal dep in ui-library (#568) (#569) * Moved frontend library from a peer dep to a normal dep in ui-library (#568) (cherry picked from commit e255bc0) # Conflicts: # Frontend/ui-library/package.json # package-lock.json * Fixing package-lock issues * Fixing issue where workspace path doesnt exist in npm publish action (#570) (#571) Trying to also fix issues with the changelog update perms. (cherry picked from commit 4f9b8a7) Co-authored-by: mcottontensor <80377552+mcottontensor@users.noreply.github.com> * Trying to fix changelog update action * Still trying to fix changeset update action * What a nightmare * Updated NPM changelogs (#572) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Fixing publish action. * Fixing publish action more. * Wrong package file. * Fix: Various input issues with input into streamed UE widgets (including IME support) (#564) (#575) * Add stub of editTextModal to application * Added events for showing onscreen keyboard, hooked them up to an edit text modal * Added explicitly stopping event propogation on text input into stream UI bleeding into UE keyboard input * Added hidden text box input for desktop IME use * Added config option to not show the modal * Added new docs page about typing into streamed inputs * Remove oncomposition event from keyboard controller as they were only needed for IME input which is now handled via modal * Removed now unused styles from PixelStreamingApplicationStyles.ts * Update Typing into streamed text inputs.md Mentioned the use of software cursor as an option. --------- Co-authored-by: Matthew.Cotton <matt@tensorworks.com.au> (cherry picked from commit 5fb6eb6) Co-authored-by: Luke Bermingham <1215582+lukehb@users.noreply.github.com> * Platform scripts audit (#567) (#577) * Limited the bash script to build cjs * Added a cjs build target * Updating bash script to be a little more friendly with builds. * Added rebuild options to bash script * Updating windows batch script to be inline with bash script. * Cleaned up arguments further for the bash script * Tweaking build behaviour. Updating args * Sorting out webpack configs (cherry picked from commit f7abd33) Co-authored-by: mcottontensor <80377552+mcottontensor@users.noreply.github.com> * Updated NPM changelogs (#576) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Luke Bermingham <1215582+lukehb@users.noreply.github.com>

* Action audit (#565) (#566) - Removing duplicated push and pull_request triggers - Removing multiple install commands when one root will work. - Removing references to UE5.5 in favor of wildcard matching (cherry picked from commit 4ee5695) Co-authored-by: mcottontensor <80377552+mcottontensor@users.noreply.github.com> * [UE5.5] Moved frontend library from a peer dep to a normal dep in ui-library (#568) (#569) * Moved frontend library from a peer dep to a normal dep in ui-library (#568) (cherry picked from commit e255bc0) # Conflicts: # Frontend/ui-library/package.json # package-lock.json * Fixing package-lock issues * Fixing issue where workspace path doesnt exist in npm publish action (#570) (#571) Trying to also fix issues with the changelog update perms. (cherry picked from commit 4f9b8a7) Co-authored-by: mcottontensor <80377552+mcottontensor@users.noreply.github.com> * Trying to fix changelog update action * Still trying to fix changeset update action * What a nightmare * Updated NPM changelogs (#572) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Fixing publish action. * Fixing publish action more. * Wrong package file. * Fix: Various input issues with input into streamed UE widgets (including IME support) (#564) (#575) * Add stub of editTextModal to application * Added events for showing onscreen keyboard, hooked them up to an edit text modal * Added explicitly stopping event propogation on text input into stream UI bleeding into UE keyboard input * Added hidden text box input for desktop IME use * Added config option to not show the modal * Added new docs page about typing into streamed inputs * Remove oncomposition event from keyboard controller as they were only needed for IME input which is now handled via modal * Removed now unused styles from PixelStreamingApplicationStyles.ts * Update Typing into streamed text inputs.md Mentioned the use of software cursor as an option. --------- Co-authored-by: Matthew.Cotton <matt@tensorworks.com.au> (cherry picked from commit 5fb6eb6) Co-authored-by: Luke Bermingham <1215582+lukehb@users.noreply.github.com> * Platform scripts audit (#567) (#577) * Limited the bash script to build cjs * Added a cjs build target * Updating bash script to be a little more friendly with builds. * Added rebuild options to bash script * Updating windows batch script to be inline with bash script. * Cleaned up arguments further for the bash script * Tweaking build behaviour. Updating args * Sorting out webpack configs (cherry picked from commit f7abd33) Co-authored-by: mcottontensor <80377552+mcottontensor@users.noreply.github.com> * Updated NPM changelogs (#576) Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Rebuilt package-lock --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Luke Bermingham <1215582+lukehb@users.noreply.github.com>

mcottontensor added 7 commits April 3, 2025 17:37

Limited the bash script to build cjs

820ed74

Added a cjs build target

8ead9b3

Updating bash script to be a little more friendly with builds.

63c8326

Added rebuild options to bash script

78953fa

Updating windows batch script to be inline with bash script.

a180038

Cleaned up arguments further for the bash script

9efc923

Tweaking build behaviour. Updating args

64df9ca

lukehb changed the title ~~Platfirm scripts audit~~ Platform scripts audit Apr 4, 2025

lukehb reviewed Apr 4, 2025

View reviewed changes

SignallingWebServer/platform_scripts/cmd/common.bat Show resolved Hide resolved

lukehb reviewed Apr 4, 2025

View reviewed changes

lukehb requested changes Apr 8, 2025

View reviewed changes

mcottontensor added auto-backport Used to specify we want a PR to auto backport to a branch, must be paired with auto-backport-to-UEX. auto-backport-to-UE5.5 labels Apr 8, 2025

Sorting out webpack configs

1bb21da

mcottontensor merged commit f7abd33 into EpicGamesExt:master Apr 8, 2025
8 checks passed

mcottontensor deleted the scripts_fix branch April 8, 2025 05:07

github-actions bot mentioned this pull request Apr 8, 2025

[UE5.5] Platform scripts audit (#567) #577

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Platform scripts audit #567

Platform scripts audit #567

Uh oh!

mcottontensor commented Apr 3, 2025

Uh oh!

changeset-bot bot commented Apr 3, 2025 •

edited

Loading

Uh oh!

Uh oh!

lukehb left a comment •

edited

Loading

Uh oh!

lukehb commented Apr 4, 2025 •

edited

Loading

Uh oh!

mcottontensor commented Apr 7, 2025

Uh oh!

lukehb commented Apr 8, 2025

Uh oh!

lukehb left a comment

Uh oh!

lukehb Apr 4, 2025 •

edited

Loading

Uh oh!

lukehb Apr 4, 2025

Uh oh!

lukehb Apr 4, 2025

Uh oh!

Uh oh!

github-actions bot commented Apr 8, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

	"build:cjs": "npx webpack --config webpack.common.js",
	"build:prod": "npx webpack --config webpack.prod.js",

Platform scripts audit #567

Platform scripts audit #567

Uh oh!

Conversation

mcottontensor commented Apr 3, 2025

Relevant components:

Problem statement:

Solution

Uh oh!

changeset-bot bot commented Apr 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

⚠️ No Changeset found

Uh oh!

Uh oh!

lukehb left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukehb commented Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mcottontensor commented Apr 7, 2025

Uh oh!

lukehb commented Apr 8, 2025

Uh oh!

lukehb left a comment

Choose a reason for hiding this comment

Uh oh!

lukehb Apr 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukehb Apr 4, 2025

Choose a reason for hiding this comment

Uh oh!

lukehb Apr 4, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

github-actions bot commented Apr 8, 2025

💚 All backports created successfully

Questions ?

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

changeset-bot bot commented Apr 3, 2025 •

edited

Loading

lukehb left a comment •

edited

Loading

lukehb commented Apr 4, 2025 •

edited

Loading

lukehb Apr 4, 2025 •

edited

Loading