Skip to content

Commit ce5a065

Browse files
authored
Merge pull request #112 from TechnologyEnhancedLearning/refactor-optimisations
Refactor optimisations
2 parents 9d81145 + f9f2e4c commit ce5a065

File tree

9 files changed

+126
-30
lines changed

9 files changed

+126
-30
lines changed

.github/PULL_REQUEST_TEMPLATE.md

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,29 @@ _Paste screenshots for all views created or changed: mobile, tablet and desktop,
2525
### Logging
2626
_Provide description of any component scoped logging or specific level logging to check
2727

28+
### Size Optimisation
29+
_Provide wasm size comparison between prod and dev showcases, does it exceed 8Mb
30+
- Network tab in Chrome
31+
- Disable cache
32+
- reload
33+
- filter wasm dll js dotnet.wasm, blazor.webassembly.js, TELBlazor.dll
34+
- if using compression check .br or .gz (if Content-Encoding: br or gz is in headers)
35+
- Sum up
36+
37+
| Measure | [Dev](https://technologyenhancedlearning.github.io/TELBlazor-DevShowCase/) Value | [Prod](https://technologyenhancedlearning.github.io/TELBlazor/) Value | Difference | Notes |
38+
|----------|--------------------------------------------------------------------------|----------------------------------------------------------------------|-------|-------|
39+
| Load Size | X.X MB | |
40+
| Lighthouse Accessibility Score | Y.Y MB | |
41+
| Difference | | |
42+
43+
44+
-----
45+
### Recommended Prepipeline steps
46+
These test are run by the pipeline but running them locally can be convenient to see issues early or to debug issues seen in the pipeline locally.
47+
- Run against release configuration by select release configuration at the the top of solution explorer (to check against optimisation such as tree shaking for example)
48+
- clean rebuild the solution
49+
- run tests against release
50+
2851
-----
2952
### Developer checks
3053
(Leave tasks unticked if they haven't been appropriate for your ticket.)
@@ -43,8 +66,10 @@ I have:
4366
- [ ] Updated my Jira ticket with testing notes, including information about other parts of the system that were touched as part of the PR and need to be tested to ensure nothing is broken
4467
- [ ] Tested in [Dev Showcase](https://technologyenhancedlearning.github.io/TELBlazor-DevShowCase/) (including logging by using log level switcher)
4568
- [ ] Scanned over my pull request and commented with any useful explanations/questions to reviewers
46-
- [ ] Scanned over cicd warnings
47-
69+
- [ ] Scanned over cicd warnings relating to the component or area of code I have worked on (give the general ones a look too but antyhing in OptionalImplementations/Test can be ignored)
70+
- [ ] Maybe? Audit NuGet packages; use lightweight ones (e.g., System.Text.Json); ensure third-party components support trimming.
71+
- [ ] Scanned in visual studio build info messages about improving code for new code
72+
- [ ]
4873
---
4974
### Peer Reviewers and Assignee checks before Approval
5075
- [ ] Feedback has been provided

.github/workflows/dev.yml

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -204,10 +204,14 @@ jobs:
204204
uses: actions/setup-node@v4
205205
with:
206206
node-version: '20'
207+
207208

208209
- name: Install npm packages so we have gulp for retrieving TEL Frontend Css
209210
run: npm ci
210211
#CI is an install that adheres to package-lock
212+
213+
- name: Install wasm-tools workload (wasm-tools used for delinking so can test against optimised client wasm using TELBlazor package)
214+
run: dotnet workload install wasm-tools --skip-manifest-update --source https://api.nuget.org/v3/index.json
211215

212216
- name: Build TELBlazor.Components (it publishes on build)
213217
run: |
@@ -301,6 +305,9 @@ jobs:
301305

302306
- name: Install npm packages so we have gulp for retrieving TEL Frontend Css
303307
run: npm ci
308+
309+
- name: Install wasm-tools workload (wasm-tools used for delinking so can test against optimised client wasm using TELBlazor package)
310+
run: dotnet workload install wasm-tools --skip-manifest-update --source https://api.nuget.org/v3/index.json
304311

305312
- name: Build solution without generating new package
306313
run: |

.github/workflows/release.yml

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -209,7 +209,9 @@ jobs:
209209
run: npm ci
210210
#CI is an install that adheres to package-lock
211211

212-
212+
- name: Install wasm-tools workload (wasm-tools used for delinking so can test against optimised client wasm using TELBlazor package)
213+
run: dotnet workload install wasm-tools --skip-manifest-update --source https://api.nuget.org/v3/index.json
214+
213215
- name: Build and pack TELBlazor.Components
214216
run: |
215217
dotnet build TELBlazor.Components -c Release \
@@ -294,8 +296,11 @@ jobs:
294296
node-version: '20'
295297

296298
- name: Install npm packages so we have gulp for retrieving TEL Frontend Css
297-
run: npm ci
298-
299+
run: npm ci
300+
301+
- name: Install wasm-tools workload (wasm-tools used for delinking so can test against optimised client wasm using TELBlazor package)
302+
run: dotnet workload install wasm-tools --skip-manifest-update --source https://api.nuget.org/v3/index.json
303+
299304
- name: Build solution without generating new package
300305
run: |
301306
dotnet build TELBlazor.sln -c Release \

.github/workflows/reuseable-ci-checks.yml

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -200,8 +200,6 @@ jobs:
200200
- name: Replace local environment variable in nuget config because cant provide it as a parameter
201201
run: |
202202
sed -i "s|%TEL_BLAZOR_PACKAGE_SOURCE%|$LOCAL_PACKAGES_PATH|g" nuget.config
203-
# sed -i "s|%GITHUB_USERNAME%|$GITHUB_USERNAME|g" nuget.config
204-
# sed -i "s|%TEL_GIT_PACKAGES_TOKEN%|$TEL_GIT_PACKAGES_TOKEN|g" nuget.config
205203
206204
- name: Clean lock files because the newly generated package file will superseed the locks
207205
run: |
@@ -239,29 +237,29 @@ jobs:
239237

240238
- name: Build and create package locally
241239
run: |
242-
dotnet build TELBlazor.Components -c Debug \
240+
dotnet build TELBlazor.Components -c Release \
243241
/p:TELBlazorPackageVersion=$TELBLAZOR_PACKAGE_VERSION \
244242
/p:NugetPackagesOutputPath=$NUGET_PACKAGES_OUTPUT_PATH \
245243
/p:UseTELBlazorComponentsProjectReference=$USE_TEL_BLAZOR_COMPONENTS_PROJECT_REFERENCE \
246244
/p:DisablePackageGeneration=false
247245
248246
- name: Build solution without generating new package
249247
run: |
250-
dotnet build TELBlazor.sln -c Debug \
248+
dotnet build TELBlazor.sln -c Release \
251249
/p:TELBlazorPackageVersion=$TELBLAZOR_PACKAGE_VERSION \
252250
/p:NugetPackagesOutputPath=$NUGET_PACKAGES_OUTPUT_PATH \
253251
/p:UseTELBlazorComponentsProjectReference=$USE_TEL_BLAZOR_COMPONENTS_PROJECT_REFERENCE \
254252
/p:DisablePackageGeneration=true
255253
256254
257255
- name: Ensure browsers are installed
258-
run: pwsh TELBlazor.Components.ShowCase.E2ETests/bin/Debug/net8.0/playwright.ps1 install --with-deps
256+
run: pwsh TELBlazor.Components.ShowCase.E2ETests/bin/Release/net8.0/playwright.ps1 install --with-deps
259257

260258

261259
- name: Run tests with coverage threshold check
262260
id: unit_e2e_tests
263261
run: |
264-
dotnet test --no-build --no-restore \
262+
dotnet test --no-build --no-restore -c Release \
265263
/p:TELBlazorPackageVersion=$TELBLAZOR_PACKAGE_VERSION \
266264
/p:NugetPackagesOutputPath=$NUGET_PACKAGES_OUTPUT_PATH \
267265
/p:UseTELBlazorComponentsProjectReference=$USE_TEL_BLAZOR_COMPONENTS_PROJECT_REFERENCE \

README.md

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -91,19 +91,22 @@ the ability to produce static prerendered html. The prerendered html is written
9191
--password $env:TEL_GIT_PACKAGES_TOKEN `
9292
--store-password-in-clear-text
9393

94-
# 3. Restore .NET CLI tools (Playwright, report manager for code coverage)
94+
#3. Install wasm-tools this allows delinking so we can have smalled wasm files producted by client projects consuming the Package. Specifying nuget to stop azure package issues.
95+
dotnet workload install wasm-tools --skip-manifest-update --source https://api.nuget.org/v3/index.json
96+
97+
# 4. Restore .NET CLI tools (Playwright, report manager for code coverage)
9598
Write-Output "Restore Tools"
9699
dotnet tool restore
97100

98-
# 4. Install Node dependencies (gulp, playwright, frontend libs)
101+
# 5. Install Node dependencies (gulp, playwright, frontend libs)
99102
Write-Output "Restore Node"
100103
npm install
101104
102-
# 5. Build solution or run other commands as needed
105+
# 6. Build solution or run other commands as needed
103106
Write-Output "Build solution without build package, using project references instead of local package or remote package"
104107
dotnet build
105108
106-
# 6. Setup playwright
109+
# . Setup playwright
107110
Write-Output "Playwright setup"
108111
& ".\TELBlazor.Components.ShowCase.E2ETests\bin\Debug\net8.0\playwright.ps1" install
109112
```
@@ -335,6 +338,8 @@ recommend tool it is designed for though both support the others tool.
335338
- not using guid id as i have elsewhere either [Parameter] public string ElementId { get; set; } = $"tel-button-{Guid.NewGuid():N}";
336339
- various things have been cut from mvcblazor so it is worth returning to for potential solutions if developing this solution [MVCBlazor repo](https://github.com/TechnologyEnhancedLearning/MVCBlazor)
337340

341+
#### Compression
342+
338343

339344
# Project Structure and Comments
340345

TELBlazor.Components.ShowCase.E2ETests.WasmServerHost/TELBlazor.Components.ShowCase.E2ETests.WasmServerHost.Client/TELBlazor.Components.ShowCase.E2ETests.WasmServerHost.Client.csproj

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,39 @@
1111
<Configurations>Debug;Release;ci</Configurations>
1212

1313
</PropertyGroup>
14+
15+
<!--Testing against Release optimisations-->
16+
<PropertyGroup Condition="'$(Configuration)' == 'Release'">
17+
18+
<!-- Trims unused code to reduce size. Can break reflection/serialization. -->
19+
<PublishTrimmed>true</PublishTrimmed>
20+
1421

22+
<!--
23+
Trim culture-specific resources to reduce WASM download size.
24+
Current reasoning: Our app only targets invariant formatting (no multi-culture UI).
25+
Trade-off: Any culture-specific number, date, or currency formatting will not work.
26+
Remove if your app needs localized formatting.
27+
-->
28+
<InvariantGlobalization>true</InvariantGlobalization>
29+
30+
<!--
31+
Trim timezone support to reduce WASM size.
32+
Current reasoning: Our app only uses UTC or fixed offsets; we don't need full client-side timezone info.
33+
Trade-off: Client-side timezone conversions will not work.
34+
Remove if your app displays local time or uses time zone conversions.
35+
-->
36+
<InvariantTimeZone>true</InvariantTimeZone>
37+
<BlazorEnableTimeZoneSupport>false</BlazorEnableTimeZoneSupport>
38+
39+
<!--
40+
Strip collation data for different cultures.
41+
Current reasoning: Only invariant string comparisons/sorting are needed, reducing payload size.
42+
Trade-off: String comparison may behave differently across cultures; e.g., sorting might be incorrect for localized strings.
43+
Remove if your app relies on culturally correct string comparisons/sorting.
44+
-->
45+
<BlazorWebAssemblyPreserveCollationData>false</BlazorWebAssemblyPreserveCollationData>
46+
</PropertyGroup>
1547
<ItemGroup>
1648
<Content Remove="Properties\launchSettings.template.json" />
1749
</ItemGroup>

TELBlazor.Components.ShowCase.E2ETests/Helpers/BrowserHelper.cs

Lines changed: 6 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -13,22 +13,14 @@ public static class BrowserHelper
1313

1414
public static async Task<IBrowserContext> CreateBrowserContextAsync(IPlaywright playwright, string browserType, bool jsEnabled, ViewportType viewport, string baseUrl)
1515
{
16-
IBrowser browser;
1716

18-
switch (browserType.ToLower())
17+
IBrowser browser = browserType.ToLower() switch
1918
{
20-
case "chromium":
21-
browser = await playwright.Chromium.LaunchAsync(new BrowserTypeLaunchOptions { });
22-
break;
23-
case "firefox":
24-
browser = await playwright.Firefox.LaunchAsync(new BrowserTypeLaunchOptions { });
25-
break;
26-
case "webkit":
27-
browser = await playwright.Webkit.LaunchAsync(new BrowserTypeLaunchOptions { });
28-
break;
29-
default:
30-
throw new ArgumentException($"Unsupported browser type: {browserType}");
31-
}
19+
"chromium" => await playwright.Chromium.LaunchAsync(new BrowserTypeLaunchOptions { }),
20+
"firefox" => await playwright.Firefox.LaunchAsync(new BrowserTypeLaunchOptions { }),
21+
"webkit" => await playwright.Webkit.LaunchAsync(new BrowserTypeLaunchOptions { }),
22+
_ => throw new ArgumentException($"Unsupported browser type: {browserType}")
23+
};
3224

3325

3426
BrowserNewContextOptions contextOptions = new BrowserNewContextOptions
@@ -42,7 +34,6 @@ public static async Task<IBrowserContext> CreateBrowserContextAsync(IPlaywright
4234
IBrowserContext context = await browser.NewContextAsync(contextOptions);
4335

4436
return context;
45-
4637
}
4738

4839
}

TELBlazor.Components.ShowCase.E2ETests/Pages/BaseComponentPages/TELButtonPageTests.cs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,7 @@ await browserContext.Tracing.StartAsync(new()
6262
// await page.WaitForSelectorAsync("button");
6363
await page.GetByRole(AriaRole.Button, new() { Name = "Click Counter" }).WaitForAsync();
6464
ILocator button = page.GetByRole(AriaRole.Button, new() { Name = "Click Counter" });
65+
await button.WaitForAsync(new() { State = WaitForSelectorState.Visible });
6566

6667
AxeResult axeResults = await button.RunAxe();
6768

TELBlazor.Components.ShowCase.WasmStaticClient/TELBlazor.Components.ShowCase.WasmStaticClient.csproj

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,38 @@
1010
<Configurations>Debug;Release;ci</Configurations>
1111
</PropertyGroup>
1212

13+
<!--Release optimisations-->
14+
<PropertyGroup Condition="'$(Configuration)'=='Release'">
15+
<!-- Trims unused code to reduce size. Can break reflection/serialization. -->
16+
<PublishTrimmed>true</PublishTrimmed>
17+
18+
19+
<!--
20+
Trim culture-specific resources to reduce WASM download size.
21+
Current reasoning: Our app only targets invariant formatting (no multi-culture UI).
22+
Trade-off: Any culture-specific number, date, or currency formatting will not work.
23+
Remove if your app needs localized formatting.
24+
-->
25+
<InvariantGlobalization>true</InvariantGlobalization>
26+
27+
<!--
28+
Trim timezone support to reduce WASM size.
29+
Current reasoning: Our app only uses UTC or fixed offsets; we don't need full client-side timezone info.
30+
Trade-off: Client-side timezone conversions will not work.
31+
Remove if your app displays local time or uses time zone conversions.
32+
-->
33+
<InvariantTimeZone>true</InvariantTimeZone>
34+
<BlazorEnableTimeZoneSupport>false</BlazorEnableTimeZoneSupport>
35+
36+
<!--
37+
Strip collation data for different cultures.
38+
Current reasoning: Only invariant string comparisons/sorting are needed, reducing payload size.
39+
Trade-off: String comparison may behave differently across cultures; e.g., sorting might be incorrect for localized strings.
40+
Remove if your app relies on culturally correct string comparisons/sorting.
41+
-->
42+
<BlazorWebAssemblyPreserveCollationData>false</BlazorWebAssemblyPreserveCollationData>
43+
</PropertyGroup>
44+
1345
<Target Name="AddNoJekyllForGitHubPagesDeployment" AfterTargets="Publish">
1446
<Touch Files="$(PublishDir).nojekyll" AlwaysCreate="true" />
1547
</Target>

0 commit comments

Comments
 (0)