GitHub Optional chaining resurgent #29
Comments
|
The Debugger shows that (eg) the script that was previously delivered with |
|
Optional chaining and nullish coalescing are both supported by current UXP. Please explain a) what breaks, b) how the cited issue is a problem on the supported platforms, and c) what this patch (don't worry about the not-PR) is supposed to fix. |
|
SeaMonkey 2.53 doesn't yet handle optional chaining (though the next .release should do). That might fall outside the supported platforms, but it would still be helpful to understand if I failed to do something obvious. Normally breaking language changes (due to incompatible JS versions being served as text/javascript' instead of application/javascript;version=ES2017` or some such) cause scripted site functionality to fail because important functions following the invalid syntax are never parsed and loaded. If the change affects dynamic behaviour rather than language parsing, some functionality may fail with a diagnostic in the error log instead. By removing unsupported language features from the delivered scripts, there should be no missing functionality, even if possibly one of the edits causes a dynamic error. However I see neither parsing errors nor dynamic errors. If this were made to work we also need in the conditional list in superseded.add("gh-script-optchain"); |
|
Ah, right. Basically your approach was correct, and last time this happened I ran into the same error: GH uses script integrity (good, actually!) so it refuses to execute the script after it was modified. Also, good catch on the typos. |
|
With some git archaeology I traced the Now I just have to fix the REs: in general nested REs would be needed but my hope that a set of REs targeting One frequent pattern is The logging feature is reassuring although because |
|
The difficulty is implement the expression short-circuits, there are too many usage modes of optional chaining. No simple way, babeljs can be competent, but too slow. |
and those scripts will load scripts with check attribute "integrity", must be rewrite as empty. |
|
This let replacer = function (mch, p1, p2) {
let chg = false;
p1 = p1.replace(/\?\.([\])])?/g, (m, p)=>(chg=true,p==null?".":p));
if (!chg) return mch;
return `((()=>{try {return ${p1};} catch (e) {return ${p2?"(()=>undefined)":"undefined"};}})())${p2==null?"":p2}`;
}
contentReplace.push(
[/((?!(?:return|eturn|turn|urn|rn|n)\()(?:[a-zA-Z_$][a-zA-Z_$0-9]*(?:\([^)]*\)|\[[^]]*\])?\??\.(?:\([^)]*\)|\[[^]]*\])?)+(?:[a-zA-Z_$][a-zA-Z_$0-9]*)?)(\()?/g,
replacer]);This generates completely unreadable (so no worse than before) but so far valid JS. I can't make SM's regex force a match that has at least one Now just these are not being processed: The optional chaining syntax seems to be matched but not replaced, even if the contentReplace is repeated. |
Indeed, it's a tool that should be deployed (probably is, or an equivalent) in GH's server build process, but they insist on setting the switch that makes ES2020 code.
If/As this isn't being done in PF, how should it be done? After further testing I have a version that's working well enough for the issue The last of these (probably all the rest too) is a script that isn't being processed even thought the script source is handled perfectly under test. The Developer Tools Browser Console (which is mysteriously different from the stand-alone display) attributes it to the TypeScript source The almost working code:
|
case "gh-script-integrity":
contentReplace.push([`.integrity=`, `._integrity=`]);
break;and outdatedmy dirty scheme (
function optchainFill(match, split, expr, target) {
return `${split}${expr}&&${expr}${"([".includes(target)?"":"."}${target}`;
}
const optchainSub = [/([,;:= {}<>!|&(\)?])((?:[a-zA-Z_$][a-zA-Z_$0-9]*|\([^()]+?\))(?:(?:\.[a-zA-Z_$][a-zA-Z_$0-9]*)*(?:\[[^[\]]+?\]|\([^()]*?\))?)*)\?\.([a-zA-Z_$]|[([])/g, optchainFill],
nullishSub = [/([a-zA-Z_$0-9\)\]])\?\?/g, "$1||"],
integritySub = [".integrity=", "._integrity="],
integritySubHtml = [/(<script[^<]+?)integrity=".+?"/g, "$1"];
/*
* special substitutions *
* behaviors
[/=(\(0[^\)]+\)\(.\.querySelector\("\.js-code-editor"\)\))\?\.editor;/, '=(([a]=[$1])=>a?a.editor:undefined)();']
* notifications-v2
[/=(.\.match\(.+?)\?\.pop\(\)\|\|"",/, '=(([a]=[$1])=>a?a.pop():"")(),']
*/ |
This comment was marked as outdated.
This comment was marked as outdated.
|
Like @SeaHOH I managed to get a working set of patches, at least to edit and label posts. Unfortunately, it's equally incomprehensible! There are some runtime JS errors logged that I expect can be fixed by reviewing the replacements, but the patched GH scripts don't throw SyntaxErrors. |
This comment was marked as resolved.
This comment was marked as resolved.
|
The point I was making is that incomprehensibility is a general characteristic of any complex regex solution (see also Perl), even with the I've posted the now apparently fully working version as above. This is the code corresponding to that posted earlier: let replacer = function (mch, p1, p2) {
let chg = false;
// only replace in context (varname|fnname(...)|arrname[...])?.(varname|(...)|[...])
if (p1 != null) {
p1 = p1.replace(/([a-zA-Z_$0-9)\]])\?\.([a-zA-Z_$([])/g, (m, q1, q2)=>(chg=true, q1 + ('[('.includes(q2)?"":".") + q2));
}
if (!chg) return mch;
// split the expression ${path}${tail} with no initial . in tail; bind `this` if necessary
// thanks https://nemisj.com/some-interesting-aspects-of-call-method-in-javascript/
const [p, e] = p1.split(/(?=(?:\.(?=[a-zA-Z_$])|[([])([a-zA-Z_$][a-zA-Z_$0-9]*|\([^)]*\)|\[[^\]]*\])$)/).concat([null]);
const try_txt = (e?
`const _1 = ${p}; const _2 = ${p1}; return (typeof _2 === 'function' && 'constructor' in _2)?_2.bind(_1):_2`:
"return " + p1);
const catch_txt = "return " + (p2 != null? "(()=>undefined)": "undefined");
const ret = `((()=>{try {${try_txt};} catch (e) {${catch_txt};}})())${p2==null?"":p2}`;
print("modifying " + mch + " to " + ret);
return ret;
}
contentReplace.push(
// match and sanitise quite a general JS access expression, rooted at identifier or (0, identifier[.identifier]*), avoiding embedded {} expressions
// not return(..., if(... identifier |\(0,[ identifier[?]. ]* identifier [(...(not {))|[...not {]]]* [?].[(...(not ))|[...not ]]]+ [identifier] (
[/(?:if\(|return\(|((?:(?:[a-zA-Z_$][a-zA-Z_$0-9]*|\(0,(?:[a-zA-Z_$][a-zA-Z_$0-9]*\??\.)*[a-zA-Z_$][a-zA-Z_$0-9]*\))(?:\((?:\$\{|[^{)])*\)|\[(?:\$\{|[^{\]])*\])*\??\.(?:\([^)]*\)|\[[^\]]*\])*)+(?:[a-zA-Z_$][a-zA-Z_$0-9]*)?))(\()?/g,
replacer]);
// const o=(0,$t.P)(t.querySelector(".js-code-editor"))?.editor
contentReplace.push(
[/(\([^)]*\)\((?:[a-zA-Z_$][a-zA-Z_$0-9]*\.)*[a-zA-Z_$][a-zA-Z_$0-9]*\([^)]*\)\)\?\.[a-zA-Z_$][a-zA-Z_$0-9]*)(\()?/g,
replacer]);Happy to PR this, but only tested on my SM 2.53.12. Enabling the add-on's debug flag and filtering for |
This comment was marked as outdated.
This comment was marked as outdated.
|
Interesting work, manually replacing code sequences with those operators with the older syntax equivalent is definitely a pain in the butt, particularly when there are multiple occurrences. BTW, I just updated to freshly released SeaMonkey 2.53.13. Release notes only mention optional chaining while nullish coalescing operator support appears to have been implemented already in 2.53.12, but its release notes don't mention it. |
|
My last attempt (to handle action logs) started going into a infinite-ish loop, so I reverted. The SM support for these features is correct, but 2.5.13 also removes some legacy experimental features that were used in popular extensions. |
|
update, Optional Chaining and Nullish Coalescing replacers were merged to function nullishOPFill(match, split, identifiers, target, operator) {
let optchain = true, expression = identifiers;
if (target) {
expression += target;
}
else {
operator = "??";
optchain = /\?\.[a-zA-Z_$[(]/.test(identifiers);
}
if (optchain) {
let async_ = expression.includes("await ");
expression = expression.replace(..._optchainSub);
expression = `${async_?"await ":""}(${async_?"async ":""}()=>{try{return ${expression}}catch(e){if(!/is (undefined|null|not a function)/.test(e.message||"")){throw e}}})()`;
}
return operator ? operator === "??" ?
`${split}![undefined,null].includes(_NP_=${expression})?_NP_:` :
`${split}[undefined,null].includes(_NP_=${expression})?undefined:_NP_${operator}` :
`${split}${expression}`;
}
const _optchainSub = [/([a-zA-Z_$0-9)\]])\?\.([([]?)/g, ((_,o,t) => `${o}${t||"."}`)],
nullishOPSub = [/([?,;:= {}()<>!|&](?!return\()|return(?=\())((?:[a-zA-Z_$][a-zA-Z_$0-9]*|\([^()]+(?:\([^()]*\))?[^()]*\))(?:(?:\??\.[a-zA-Z_$][a-zA-Z_$0-9]*)*(?:\[[^[\]]+?\]|\((?:[^()]+\([^()]*\))?[^()]*\))?)*)(?:\?\?|(\?\.(?:[a-zA-Z_$][a-zA-Z_$0-9]*|\[[^[\]]+?\]|\((?:[^()]+\([^()]*\))?[^()]*\))(?:(?:\??\.[a-zA-Z_$][a-zA-Z_$0-9]*)*(?:\[[^[\]]+?\]|\((?:[^()]+\([^()]*\))?[^()]*\))?)*)([([]|\?\?)?)/g, nullishOPFill],
// "_NP_" MUST be declared, front of main HTML is OK
nullishOPSubHtml = ["<link", `<script>var _NP_;</script>\n<link`],
integritySub = [".integrity=", "._integrity="],
integritySubHtml = [/(<script[^<]+?)integrity=".+?"/g, "$1"];applied scripts: |
|
https://github.com/dirkf/palefill/releases/tag/v1.19df is working for me. |
|
it's great that palefill, unlike youtube-dl, sees regular releases 👍 |
|
You are looking at a tool dependency! |
martok
added
wontfix
See JustOff/github-wc-polyfill#68 (comment) for an example of this nonsense.
So I thought it should be possible to apply an edit to the GH scripts using this nice extension. If they say a.b.c?.d, we should be able to replace that with
((_x=>(_x==null)?undefined:_x.d)(a.b.c)). There was already a minor example in thegh-script-optchainfix (which I renamed below after removing that).I edited
lib/main.js, also removing an undefined diagnostic forfixes.a:and
lib/builtin-rules.js:(Anyone who asks why this isn't a GH PR should try making one when GH scripts aren't working)
Now, I've abolished the console errors, but apparently the scripts are failing silently. I'm getting this
None of the "sha512" hashes in the integrity attribute match the content of the subresource.apparently from the
catchat the end ofTracingListener.onStopRequest.Any advice?
The text was updated successfully, but these errors were encountered: