-
-
Notifications
You must be signed in to change notification settings - Fork 111
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
change(web): lm-worker bundling + wrapper script rework 📜 #10264
Merged
Merged
Changes from all commits
Commits
Show all changes
12 commits
Select commit
Hold shift + click to select a range
154cac8
chore(web): drops unused index.js artifact from lm-worker
jahorton 0653398
chore(web): mild lm-worker bundling simplification
jahorton ad56b9e
chore(web): centralizes lm-worker build polyfill handling
jahorton 2abf2c0
change(web): lm-worker wrapper generalization prep
jahorton 0e20cc3
chore(web): a bit more lm-worker wrapping polish
jahorton 9f2816c
change(web): lm-worker wrapper script generalization
jahorton 7392fdd
refactor(web): lm-worker polyfiller rework
jahorton 17605a4
change(web): lm-worker build dir constants
jahorton 1e6f976
chore(web): adds low-arg warning to scripts
jahorton 800231d
chore(web): Merge branch 'master' into change/web/lm-worker-bundling-…
jahorton 6c38e45
chore(web): Apply suggestions from code review
jahorton d230a03
chore(web): Merge branch 'master' into change/web/lm-worker-bundling-…
jahorton File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
This file was deleted.
Oops, something went wrong.
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,112 @@ | ||
import fs from 'fs'; | ||
|
||
import convertSourcemap from 'convert-source-map'; // Transforms sourcemaps among various common formats. | ||
// Base64, stringified-JSON, end-of-file comment... | ||
|
||
let INCLUDE_SRCMAPS = false; | ||
|
||
let sourceFromArgs; | ||
let destFromArgs; | ||
|
||
function doHelp(errMessage) { | ||
if(errMessage) { | ||
console.error(errMessage + '\n'); | ||
} | ||
|
||
console.log(` | ||
Summary: | ||
Creates a "wrapped" version of the lm-worker for compilation into and inclusion as part | ||
of a different JS bundle for later use as the core of a predictive-text WebWorker. | ||
|
||
Usage: | ||
node build-wrapper.js <input-file> [options...] | ||
|
||
Parameters: | ||
<input-file>: Fully-bundled and compiled JS file to be wrapped. | ||
|
||
Options: | ||
--help Shows this script's documentation | ||
--out <out-file> Specifies the destination path for the wrapped output. | ||
|
||
If missing, the output will be placed next to the source and given | ||
the same path, but with '.wrapped.js' replacing the original '.js' | ||
extension. | ||
--sourceMap Includes the script's original sourcemaps within the wrapped output | ||
` ); | ||
process.exit(errMessage ? 1 : 0); | ||
} | ||
|
||
if(process.argv.length > 2) { | ||
for(let i = 2; i < process.argv.length; i++) { | ||
const arg = process.argv[i]; | ||
|
||
switch(arg) { | ||
case '--help': | ||
doHelp(); | ||
break; | ||
case '--sourceMap': // bc TS uses this exact flag. esbuild... uses sourcemap (in the JS config) | ||
jahorton marked this conversation as resolved.
Show resolved
Hide resolved
|
||
case '--sourcemap': | ||
case '--source-map': | ||
INCLUDE_SRCMAPS = true; | ||
break; | ||
case '--out': | ||
destFromArgs = process.argv[++i]; | ||
break; | ||
default: | ||
if(!sourceFromArgs) { | ||
sourceFromArgs = arg; | ||
} else { | ||
doHelp("Input file can only be specified once; aborting"); | ||
} | ||
} | ||
} | ||
} else { | ||
// Display help + abort. | ||
doHelp("Required parameters missing"); | ||
} | ||
|
||
if(!sourceFromArgs || sourceFromArgs.substring(sourceFromArgs.length - 3) != '.js') { | ||
doHelp("No input file has been specified; aborting."); | ||
} | ||
|
||
const sourceFile = sourceFromArgs; | ||
const destFile = destFromArgs || sourceFromArgs.substring(0, sourceFromArgs.length - 3) + '.wrapped.js'; | ||
|
||
// Now, to build the wrapper... | ||
|
||
const script = fs.readFileSync(sourceFile); | ||
|
||
// Wrapped in a function so we can leverage `const` with the result. | ||
function buildSrcMapString() { | ||
const sourcemapJSON = convertSourcemap.fromJSON(fs.readFileSync(`${sourceFile}.map`)).toObject(); | ||
const encodedSrcMap = convertSourcemap.fromObject(sourcemapJSON).toBase64(); | ||
return `//# sourceMappingURL=data:application/json;charset=utf-8;base64,${encodedSrcMap}`; | ||
} | ||
|
||
// While it IS possible to do partial sourcemaps (without the sources, but with everything else) within the worker... | ||
// the resulting sourcemaps are -surprisingly- large - larger than the code itself! | ||
const srcMapString = INCLUDE_SRCMAPS ? buildSrcMapString() : ""; | ||
|
||
/* | ||
* It'd be nice to do a 'partial' encodeURIComponent that only gets the important bits... | ||
* but my attempts to do so end up triggering errors when loading. | ||
*/ | ||
|
||
let rawScript = script.toString(); | ||
// Two layers of encoding: one for the raw source (parsed by the JS engine), | ||
// one to 'unwrap' it from a string _within_ that source. | ||
let jsonEncoded = JSON.stringify(rawScript); | ||
|
||
let wrapper = ` | ||
// Autogenerated code. Do not modify! | ||
// --START:LMLayerWorkerCode-- | ||
|
||
export var LMLayerWorkerCode = ${jsonEncoded}; | ||
|
||
${!INCLUDE_SRCMAPS && "// Sourcemaps have been omitted for this release build." || ''} | ||
export var LMLayerWorkerSourcemapComment = "${srcMapString}"; | ||
|
||
// --END:LMLayerWorkerCode | ||
`; | ||
|
||
fs.writeFileSync(destFile, wrapper); |
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
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks like it might be flipped in 18.0? Do we need an issue to track this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Assuming I'm properly understanding you here, we will not need an issue to track this. From #10257:
keyman/common/web/es-bundling/src/configuration.mts
Lines 24 to 35 in dc7bfc1
The ES6 target's configuration will re-enable this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will we ever want
keepNames: false
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's not viable on Android 5.0 devices, so yes.
https://caniuse.com/mdn-javascript_builtins_function_name_configurable_true