Skip to content

Snapshot-friendly#162

Merged
PeterStaev merged 4 commits intoPeterStaev:masterfrom
DickSmith:snapshot-friendly
Feb 12, 2018
Merged

Snapshot-friendly#162
PeterStaev merged 4 commits intoPeterStaev:masterfrom
DickSmith:snapshot-friendly

Conversation

@DickSmith
Copy link
Copy Markdown
Contributor

Changes to allow this plugin to be used with android webpack/snapshot.

Java package/namespaces usage evaluated at build time will cause snapshot to fail so must be placed in a function that will not execute until runtime. Additional typing added via interfaces to compensate for the loss of having the class inside a function.

@PeterStaev
Copy link
Copy Markdown
Owner

Hey @DickSmith , can you explain in which cases doe snapshot fail? Because the current demo and demo-ng apps are compiled with snapshot during CI and from what I see there are no problems (https://travis-ci.org/PeterStaev/NativeScript-Drop-Down/jobs/340329407#L3645). So seems there is some specific use case where snapshots are failing and I want to include those in the demo apps so it is properly tested by TravisCI.

Thanks!

Comment thread .gitignore Outdated
!webpack.*.js
report

package-lock.json
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can safely add the package-lock.json to git.

@DickSmith
Copy link
Copy Markdown
Contributor Author

DickSmith commented Feb 12, 2018

Hey @PeterStaev,

So snapshot only takes a snapshot of the vendor bundle (by default), so plugins will only get snapshotted if you add import 'nativescript-drop-down' (TS-style) or require('nativescript-drop-down') (JS-style) to app/vendor.ts.

So in looking at the vendor.ts for both demo and demo-ng it looks like the plugin isn't actually being snapshotted. If you add the plugin there (without my changes) you should see it fail snapshot complaining about ReferenceError: android is not defined or for java depending on which packages are being used. Otherwise nativescript-drop-down doesn't get included in the snapshot, which it ideally is if used in multiple modules (like I do in my form-heavy app :D ).

I had to fork and make similar changes for ~8 plugins to be able to make snapshot work with my current vendor.ts imports. Totally worth it, though, as I shaved ~50% off my boot times (from about ~6 seconds down to ~4 seconds on my own LG G6, with similar ratios on even slower devices).

For reference I made a similar PR to nativescript-background-http.

I've added now added the package-lock.json to the repo with my latest commit and also added require("nativescript-drop-down") to demo-ng (for PAN apps I'm not sure if it should be added before, after, or in-between .vendor-platform and bundle-entry-points so you'd have to find out what the best practice is there).

@PeterStaev PeterStaev merged commit 0b6b011 into PeterStaev:master Feb 12, 2018
@PeterStaev
Copy link
Copy Markdown
Owner

Very interesting! I might have to look at my apps and add some of the plugins to the vendor chunk 😉 Thanks for your work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants