feat(web): alternate artifact - es6-bundled Web 📜 #10257
Conversation
keymanapp-test-bot
bot
added
the
user-test-missing
User Test ResultsTest specification and instructions User tests are not required Test Artifacts
|
| ".": { | ||
| "es6-bundling": "./src/index.ts", | ||
| "default": "./build/obj/index.js" | ||
| }, |
There was a problem hiding this comment.
Many thanks to evanw/esbuild#1250 (comment) for the approach used in this PR.
Leveraging this feature during TypeScript compilation requires TS 5.0: https://www.typescriptlang.org/docs/handbook/release-notes/typescript-5-0.html#customconditions
We're currently using 4.9.5, so we can't leverage that during ES5 builds... not that we've needed to, anyway.
| @@ -79,5 +82,6 @@ | |||
| "src/schemas/*", | |||
| "test/" | |||
| ] | |||
| } | |||
| }, | |||
| "sideEffects": false | |||
There was a problem hiding this comment.
Without this specific line, a lot of the package fails to tree-shake out. Notably, all of the precompiled schemas would get included, which would cause major filesize bloat.
While the linked comment talks about JS imports, rather than explicitly-TS imports in the manner I've set up here... the same concerns appear to apply.
Perhaps a package used by common/web/types doesn't include the sideEffects package tag, and that's cascading? Just a hunch... at the moment. I do see that there are three external non-dev dependencies that it uses... and one of them has its own dependencies. (None of the three are marked with sideEffects: false, either - but doing this isn't enough.)
I've tried investigating this... but without much luck. All I can say is that something... seems "off" with any attempt to treeshake common/web/types without this package's flag.
The most relevant line of thought I can think of for now: the schema validators are spit out as .mjs files, not TS, making that comment above more relevant. That said, if I locally comment out all Web-ignored exports from common/web/types/src/main.ts, then re-enable the export * as util from './util/util.js', that export doesn't fully tree-shake out, and it doesn't refer to the schemas, so... yeah.
Not sure if it's relevant, but I do see a mild difference with "sideEffects": false for the gesture-recognizer - the subfolder index.ts files - for a common export interface - get tree-shaken out with the flag, but do not when the flag is missing. For the gesture-engine... it makes all of 0.3kb difference.
Does not affect the worker. Bundling is in a temp-script and not properly connected to our actual build infrastructure.
jahorton
changed the title
jahorton
changed the base branch from
master
to
change/web/build-bundler-replacement
jahorton
force-pushed
the
feat/web/es6-target-artifact
branch
from
896eb61
to
ba3dbbe
Compare
keymanapp-test-bot
bot
removed
the
user-test-missing
…feat/web/es6-target-artifact
|
Changes in this pull request will be available for download in Keyman version 17.0.239-alpha |
KeymanWeb's main artifact may now be built targeting ES5 (though needing polyfills) or ES6 (without), the latter of which gives a pretty significant filesize savings.
app/webviewwill follow with its own set of changes, as I ran into an interesting complication there worth investigating. It's also important to avoid accidentally forcing use of a ES6 artifact for all devices before we get the ES5/ES6 selection code ready for the Android app.First commit results
New
Contrast with the feature-branch's current size:
Old
Second commit results
New
Contrast with the feature-branch's current lm-worker size:
Old
Also keep in mind: for a top-level es6 target, we should be able to wholesale drop the polyfills that bloats it to this size in main Web:
Old
Of note: again, so far these scripts are not connected to the full Web toolchain. They do give us useful information on how many bytes we can expect to 'save', though. I also am not yet building the wrapped version of the es6-bundled worker.
Also of note: these changes do not affect the standard Web build. That said, I haven't yet run any tests or unit tests on either the old es5-oriented artifact or the new, potential es6 artifacts.
Also of note:
master!As of b95051b:
Targeting ES5:
Targeting ES6:
Note that this commit's build checks have been performed against the ES6 artifact, not the ES5 one!
@keymanapp-test-bot skip
It's exceedingly unlikely that this would break KMW without it also breaking at least one of the automated tests. There was one break, but it did cause auto-test failure.