Repository navigation
fix(android): load prebuilt classes for constructor-less and dollar-named extends - #135
Open
ammarahm-ed wants to merge 3 commits into
Open
ammarahm-ed wants to merge 3 commits into
ammarahm-ed wants to merge 3 commits into
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
|
This branch has not been deployed
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Ports the parts of NativeScript/android#2101 that apply to this tree, so more
extendand@NativeClassclasses are loaded from the APK instead of being generated on the device.Changes
return _super !== null && _super.apply(this, arguments) || this;, readable and minified, so a TypeScript class without a constructor gets a file, line and column instead of empty onesDexFactory.resolveClasstries the class name as given before the$→_form, so a prebuilt class whose file name contains$is foundLocale.USso it passes on locales that use,as the decimal separatorNot ported
ExtendFileIdentifierand the md5 in the file name: this tree's generator names classes<Base>_<last path part>_<line>_<col>_<name>, which is whatGetExtendLocationalready produces in both runtimes, so there is no name mismatchgenerateDex: the cache key here is<name>-<thumb>.dexon both the write and the lookup side.mjsor HTTP modulesBefore / after
The new jsparser case
extends_ts_no_constructorfails without the change (the generated file name isno_constructor***Minified, with empty line and column) and passes with it: 13 jsparser specs, 0 failures.The device specs in
testPrebuiltExtendClasses.jspass with and without theDexFactoryand parser changes on this test app (533 specs, 0 failures both ways), so they are regression guards, not proof of the fix. No cold-start numbers: the naming and cache fixes behind upstream's 6% do not apply here.Tests
Jasmine suite on a Sony SO-01M with V8: Node-API 533 specs, 0 failures; direct 461 specs, 0 failures. Runtime Java unit tests pass.
static-binding-generatorGeneratorTest(ClassFormatError) andNodeJSProcessImplTestfail on the stack as well and are unchanged.Expected locations in the new parser case are columns 64 and 108 because this parser reports the call's end column for this shape, as it already does for
var _this = _super.call(this) || this.Stacked on #134.