JavaScript port: wire the native themes from Themes/, drop unused demo fixtures#5466
JavaScript port: wire the native themes from Themes/, drop unused demo fixtures#5466shai-almog wants to merge 5 commits into
Conversation
JavaScriptPort.jar carries webapp/ so off-repo builds can find port.js, and the translator copies webapp/assets into every app it builds. That meant ~3.6MB of the port test app's own fixtures -- leather.res (2.1MB), chrome.res (778KB), video.mp4 (666KB), Handlee-Regular.ttf, Page.html, test.json -- landed in every JavaScript app's public web root. None of them is referenced by any class in the port or in CodenameOne core. Exclude them from the jar. They stay in the source tree, so in-repo builds and the javascript-screenshots suite still see them. The system themes HTML5Implementation.getNativeTheme() resolves (iOS7Theme.res, iPhoneTheme.res, iOSModernTheme.res, android_holo_light.res, tzone_theme.res) and CN1Resource.res are kept. Also delete the jar before rebuilding it: Ant's jar task treats an existing archive as up to date when it is newer than every input file, so on an incremental build a change to this include/exclude list is silently ignored and a stale jar ships. That cost a full debugging cycle here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR reduces the default output size of JavaScript builds by preventing the JavaScript port’s demo/test fixture assets from being bundled into JavaScriptPort.jar, which the translator then propagates into every JavaScript app’s public web root. It also makes the jar packaging step robust for incremental builds by forcing the jar to be recreated when include/exclude rules change.
Changes:
- Delete the previously-built
JavaScriptPort.jarbefore running Ant’s<jar>task to avoid stale archives on incremental builds. - Exclude large demo/test fixture files (e.g.,
leather.res,chrome.res,video.mp4) from thewebapp/assetscontent that gets embedded intoJavaScriptPort.jar.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| The system themes assets/ must keep are the ones | ||
| HTML5Implementation.getNativeTheme() resolves: | ||
| iOS7Theme.res, iPhoneTheme.res, iOSModernTheme.res, | ||
| android_holo_light.res and tzone_theme.res. --> |
✅ Continuous Quality ReportTest & Coverage
Static Analysis
Generated automatically by the PR CI workflow. |
assets/theme.res is the port test app's theme (1.4MB). Real apps ship their own theme.res at the bundle root and getResourceAsStream() deliberately prefers the root copy for theme.res / CN1Resource.res, so this one is never read -- confirmed against a running app, whose request log shows the root theme.res fetched and this one never touched. Shipping it also kept a shadowing hazard alive for no benefit. assets/tzone_theme.res is returned by no code path: getNativeTheme() only ever yields iOS7Theme / iPhoneTheme / iOSModernTheme / android_holo_light / AndroidMaterialTheme / androidTheme. The single mention of tzone_theme in the port is inside a comment, which is exactly why it survived the first pass. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
maven/parparvm/pom.xml:242
- The comment listing the “system themes assets/ must keep” includes
tzone_theme.res, but the JS port’s theme selection doesn’t return that resource. InPorts/JavaScriptPort/.../HTML5Implementation.java,resolveNativeThemeResource()returns only iOS7/iPhone/iOSModern/AndroidMaterial/android_holo_light/androidTheme (see ~4092-4147), so this list is misleading and contradicts the later exclusion ofassets/tzone_theme.res. Update the comment to reflect the actual set of themes that may be loaded.
The system themes assets/ must keep are the ones
HTML5Implementation.getNativeTheme() resolves:
iOS7Theme.res, iPhoneTheme.res, iOSModernTheme.res,
android_holo_light.res and tzone_theme.res. -->
maven/parparvm/pom.xml:261
- This exclusion rationale refers to
getNativeTheme(), but there is nogetNativeTheme()in the JavaScript port; theme selection is performed byHTML5Implementation.resolveNativeThemeResource()(called frominstallNativeTheme()). Updating this comment will avoid confusion when someone tries to audit which themes must remain bundled.
<!-- No code path returns tzone_theme.res: getNativeTheme()
only ever yields iOS7Theme / iPhoneTheme /
iOSModernTheme / android_holo_light /
AndroidMaterialTheme / androidTheme. The one mention of
tzone_theme in the port is inside a comment. -->
I had described iOS7Theme/iPhoneTheme/android_holo_light as "the themes getNativeTheme() resolves" and listed iOSModernTheme among them. That misread installNativeTheme(): the JS port defaults to the LEGACY pair (android_holo_light on an Android user agent, iOS7Theme elsewhere) so existing screenshot baselines stay comparable, and the modern themes are opt-in via ios.themeMode / and.themeMode / nativeTheme / javascript.native.theme. The modern .res files live in Themes/ and are staged per target, not from webapp/assets. No functional change: the same files are excluded and the same ones kept. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
maven/parparvm/pom.xml:243
- PR description says the bundle keeps
tzone_theme.resandtheme.res, but this POM change explicitly excludes both (assets/theme.resandassets/tzone_theme.res). Either the description/kept list is outdated, or these excludes are too aggressive. Please align the PR description and/or the inline comment here so it’s clear which themes/resources are intentionally retained vs excluded (and why).
Kept: the native themes installNativeTheme() can open
from the bundle. The JS port defaults to the LEGACY
pair (android_holo_light.res on an Android user agent,
iOS7Theme.res elsewhere) so existing screenshot
baselines stay comparable; iPhoneTheme.res is the
HTML5Implementation.resolveNativeThemeResource() can return six themes -- iOS7Theme, iPhoneTheme, iOSModernTheme, androidTheme, android_holo_light, AndroidMaterialTheme -- but the bundle only carried three of them, so setting ios.themeMode / and.themeMode / nativeTheme / javascript.native.theme to anything modern (or to legacy on Android) hit the catch in installNativeTheme() and silently fell back to the legacy theme. The modern themes postdate the old TeaVM port; the ParparVM port never grew the wiring the other targets have. The build cannot pick one of the six: resolveNativeThemeResource() decides at RUNTIME from the browser user agent, so "modern" means iOSModernTheme in an iOS-like browser and AndroidMaterialTheme everywhere else. Ship the whole set instead. Take them from Themes/, the single source of truth kept in sync from the CSS sources by native-themes-sync.yml, the same way maven/ios folds iOSModernTheme.res into nativeios.jar and maven/javase copies the set onto its classpath -- rather than from the port's own copies under webapp/assets, which drift: the webapp's iPhoneTheme.res was already a stale version of the Themes/ one. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
shai-almog
changed the title
|
Scope grew after review feedback — this now fixes a real bug, not just bloat. The modern native themes were never wired up on the JavaScript port. They postdate the old TeaVM port, and the ParparVM port never grew the staging the other targets have. Note the build cannot stage just the one an app needs: They now come from Net size: the six themes add ~780KB versus the three stale copies, and removing the demo fixtures takes ~3.6MB off, so a JavaScript bundle still ends up substantially smaller than before. I also closed #5468, which I had filed claiming the modern path was broken by default. That was wrong — legacy is the intended default, to keep screenshot baselines comparable. The real defect is the one fixed here: the opt-in never worked. |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
maven/parparvm/pom.xml:263
- This change excludes assets/tzone_theme.res from the shipped JavaScriptPort.jar, but (a) the PR description says it is kept, and (b) apps can still explicitly request it via Display property javascript.native.theme. With this exclusion, that explicit selection will now fall back to the legacy theme at runtime.
<!-- No theme-resolution path yields tzone_theme.res; the one
mention of it in the port is inside a comment. -->
<exclude name="assets/tzone_theme.res"/>
|
Compared 151 screenshots: 151 matched. Native Android coverage
✅ Native Android screenshot tests passed. Native Android coverage
Benchmark ResultsDetailed Performance Metrics
|
|
Compared 144 screenshots: 144 matched. |
|
Compared 149 screenshots: 149 matched. Benchmark Results
Build and Run Timing
Detailed Performance Metrics
|
The bundle is meant to be unpacked straight into a web root, but two things made every consumer post-process it first. Zip the dist FLAT. zipDirectory() was passed the dist directory's own name as the zip root, so everything landed under "<MainClass>-js/" and had to be flattened before it could be served. The old cloud/TeaVM bundle was flat, so this also lets one deployment script handle both. Stop copying build descriptors into the bundle. ByteCodeTranslator copies every non-class input file into the output by basename, so a Maven-built app leaks META-INF/maven/**/pom.xml -- dependency list and all -- plus pom.properties and MANIFEST.MF. Harmless for the iOS target, where the output is a source tree; for the JavaScript target that output IS a public document root. None of them is an application resource. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Two more build-side fixes, prompted by the fair question of why a consumer should have to post-process the bundle at all. The JavaScript build is supposed to hand back something deployable as-is, and it wasn't. 1. The zip is now flat. 2. Build descriptors no longer leak into the bundle. Both were being worked around downstream; that workaround is now deleted. Two things I deliberately did not change, since they look intentional and you'd know better:
Verified end to end after both changes: the bundle unzips straight into a web root, the console renders, all APIs return 200, and a request log from the running app matches the shipped file list exactly. |
Follow-up to #5465 / #5457.
JavaScriptPort.jarcarrieswebapp/so off-repo builds can findport.js(that was #5457), and the translator copieswebapp/assetsinto every app it builds. The side effect is that the port test app's own fixtures ship inside every JavaScript app's public web root:leather.reschrome.resvideo.mp4Handlee-Regular.ttfPage.html,test.json~3.6 MB, none of it referenced by any class in the port or in CodenameOne core. I checked each name with fixed-string searches across
Ports/JavaScriptPort/src/main/javaandCodenameOne/src— zero hits. (Handlee-Regular.ttfappears only inside a Javadoc example inFont.java.)They are excluded from the jar and stay in the source tree, so in-repo builds and the
javascript-screenshotssuite still see them.Kept
The system themes
HTML5Implementation.getNativeTheme()actually resolves —iOS7Theme.res,iPhoneTheme.res,iOSModernTheme.res,android_holo_light.res,tzone_theme.res— plusCN1Resource.resandtheme.res.Also: force the jar to be recreated
Ant's
<jar>treats an existing archive as up to date when it is newer than every input file. The include/exclude list lives in the POM, not in the inputs, so on an incremental build a change to it is silently ignored and a stale jar ships. This cost a full debugging cycle: the exclusions above appeared to do nothing until I noticed the inner jar was hours old. The task now deletes the jar first.Verification
Rebuilt the bundle and inspected the jar directly: 0 demo fixtures, 4 system themes retained. Then rebuilt a real app (the BuildCloud console) against it — bundle 16 MB → 12 MB, and it still renders with icons, theme and material icon font intact, all APIs 200, browser console clean.
🤖 Generated with Claude Code