Skip to content
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

Doesn't work with --browserify option #1616

Closed
stalniy opened this issue Aug 5, 2017 · 15 comments
Closed

Doesn't work with --browserify option #1616

stalniy opened this issue Aug 5, 2017 · 15 comments
Labels

Comments

@stalniy
Copy link
Contributor

stalniy commented Aug 5, 2017

I'm submitting a ... (check one with "x")
[ ] question
[x] any problem or bug report
[ ] feature request

plugin version: (check one with "x")
[ ] 1.4.x
[x] 2.0.0-beta2

Current behavior:

$ cordova build ios --emulator --browserify
Error: 'return' outside of function (5:2) (while aliasify was processing /home/sergii/projects/misto/plugins/cdv-googlemaps/www/googlemaps-cdv-plugin.js) while parsing file: /home/sergii/projects/misto/plugins/cdv-googlemaps/www/googlemaps-cdv-plugin.js

Expected behavior:

it should be possible to create browserified build

@wf9a5m75
Copy link
Member

wf9a5m75 commented Aug 5, 2017

If you create it, and send it, I will consider it.

@wf9a5m75
Copy link
Member

wf9a5m75 commented Aug 5, 2017

I think this common sense, the person who only do requests is not liked.

At least, --browserify does not support at this time.

If you want to use it, you should try that by yourself at least.

@stalniy
Copy link
Contributor Author

stalniy commented Aug 5, 2017

what do you mean? I didn't get, sorry

@wf9a5m75
Copy link
Member

wf9a5m75 commented Aug 6, 2017

Please explain why I should need to work for --browserify? What is the benefit for me?
At least, no benefit for me, and nobody requested about this so far.

I give you (not only you, general people) this plugin code as free.
Then you request the --browserify option should work.
So the person who would get benefit with this option is only for you.
Why should I work for you?
Did you try to fix the problem?

@stalniy
Copy link
Contributor Author

stalniy commented Aug 6, 2017

Sorry my bad. Again this is about performance. I will try to answer on all of your questions

Benefits

I will explain benefits based on my app setup. My app uses bunch of cordova plugins:

cdv-googlemaps 3.0.0-alpha-20170804-1948 "cdv-googlemaps"
com.googlemaps.ios 2.3.0 "Google Maps SDK for iOS"
cordova-plugin-camera 2.4.1 "Camera"
cordova-plugin-compat 1.1.0 "Compat"
cordova-plugin-crosswalk-webview 2.3.0 "Crosswalk WebView Engine"
cordova-plugin-file 4.3.3 "File"
cordova-plugin-file-transfer 1.6.3 "File Transfer"
cordova-plugin-filepath 1.0.2 "FilePath"
cordova-plugin-safariviewcontroller 1.4.7 "SafariViewController"
cordova-plugin-splashscreen 4.0.3 "Splashscreen"
cordova-plugin-statusbar 2.2.3 "StatusBar"
cordova-plugin-whitelist 1.3.2 "Whitelist"
cordova-plugin-wkwebview-engine 1.1.4-dev "Cordova WKWebView Engine"
ionic-plugin-deeplinks 1.0.15 "Ionic Deeplink Plugin"
ionic-plugin-keyboard 2.2.1 "Keyboard"

Each of them has 1 or more js files. So, the default cordova emulate android results in this load times (in ms, used performance.now() and iOS emulator for testing):

scripts: 61 plugin script + cordova.js + 3 app scripts = 65  requests for script sources
window load event -  – 5726
deviceready -  – 4807.2
DOMContentLoaded event -  – 1529.3

On the other hand cordova emulate android --browserify:

scripts: cordova.js + 3 app scripts = 4 blocking requests for script sources but now we are also able to add `async` and `defer` attributes to `<script src="cordova.js" async defer></script>` and now we have only 3 blocking requests!
window load event - 1033.5
deviceready - 1160.6
DOMContentLoaded event - 661

Performance boost in ~5 times for deviceready event :)

What is browserify?

In web frontend development minification and file concataion play a crucial role. Less files to request you have, faster your page is loaded.

For example, you have 20 files. You can add all of them into index.html using <script> tag and it will work but browser will send 20 requests and will wait for all that 20 requests to finish. Browserify allows to merge all files into single file (or few bundles) and intead of 20 requests, send 1 request.

In context of cordova applications cordova build ios --browserify allows to merge platform related javascript files and all plugin javascript files into single cordova.js file.

Why has nobody asked before about this feature?

  1. I guess people just don't know :)
    Cordova documentation is not good enough. If you go to CLI documentation and find --browserify option it says

Compile plugin JS at build time using browserify instead of runtime

This sounds completely unclear for me but if it would be written like:

Compiles all plugins JS files into single file and improves application boot time

I'd be more interested in this feature :)

  1. Those who use wk-web-view may find a cordova bug - https://issues.apache.org/jira/browse/CB-11311 which says that WKWebView plugin is not compatible with --browserify option and just leave everything as is. Yesterday I created a hook which can be used as workaround to WKWebView browseify issue and my ionic2 app boots much faster (details a bit later).

The Fix

The fix is extremely trivial. What we need to do is to remove 3 lines of code :) So, I can create PR in few minutes. The issue is somebody wrote a return statement outside of function. It's not allowed in javascript. I guess --browserify does some static analyses and that's why fails with error Error: 'return' outside of function

Don't know why somebody put that logic inside but I don't think that somebody will use that file outside of cordova env it just doesn't make sense. For testing purposes it's possible to mock cordova and all related methods

Why did I create issue?

I didn't have time to solve it yesterday and thought that you may consider to fix it. It's exteremely easy to fix and resulting performance boost is very good :)

@stalniy
Copy link
Contributor Author

stalniy commented Aug 6, 2017

Also this allows to uglify resulting cordova.js with after_build hook and gain even better load time

@wf9a5m75
Copy link
Member

wf9a5m75 commented Aug 6, 2017

Okay, I understand your point.
I'm testing on my way.

@wf9a5m75
Copy link
Member

wf9a5m75 commented Aug 6, 2017

If I remove the 3 lines, then test on Android, works fine.
But on iOS, I got an error for some reason.
screen shot 2017-08-06 at 11 51 40 am

@stalniy
Copy link
Contributor Author

stalniy commented Aug 6, 2017

you use WKWebView and there is an issue https://issues.apache.org/jira/browse/CB-11311 . I created a workaround hook which fixes this issue - https://gist.github.com/stalniy/90202f09c6ededff54b959e79ae3962e

Then you need to update your config.xml:

    <platform name="ios">
        <hook src="path/to/hooks/fix-browserify-wkwebview.js" type="after_build" />

@wf9a5m75
Copy link
Member

wf9a5m75 commented Aug 6, 2017

Well, I tried couple of times, but still get the error.

@stalniy
Copy link
Contributor Author

stalniy commented Aug 6, 2017

could you please provide a link to repo or steps to reproduce?

@wf9a5m75
Copy link
Member

wf9a5m75 commented Aug 6, 2017

@wf9a5m75
Copy link
Member

wf9a5m75 commented Aug 6, 2017

Build log
log.txt

@wf9a5m75
Copy link
Member

wf9a5m75 commented Aug 8, 2017

I update the code.

@stalniy
Copy link
Contributor Author

stalniy commented Aug 8, 2017

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants