cordova-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Ian Clelland <iclell...@chromium.org>
Subject Re: Browserify JS is in
Date Fri, 12 Dec 2014 03:46:21 GMT
On Thu Dec 11 2014 at 9:29:08 PM Anis KADRI <anis.kadri@gmail.com> wrote:

> On Thu, Dec 11, 2014 at 1:45 PM, Michal Mocny <mmocny@chromium.org> wrote:
> >
> > 25MB is for the one-time cordova-cli install, and not overhead for the
> > app.  Its not perfect but not a blocker imho.
>

Agreed. In terms of size for a developer tool, that's not too bad. And if
for that we get a community, bug fixes, more eyes, and fixes for the edge
cases that we haven't come across yet, then it's a pretty small price.


> >
> > There are more troubling things about the change than the size overhead:
> > - I don't think browserify has "baked" at all.  It was landed behind a
> > flag, but is it actually used anywhere?
> > - Why browserify and not webpack (aside: its 13Mb..)?  Was it compared at
> > all..
>

> - Last time it was discussed, there were still several breaking changes
> for
> > plugins.  Were they resolved?
> >
> > Anyway, how can we get data on the change to see if it is positive?:
> > - Existing plugins continue to work (We run mobile spec, and get
> downstream
> > distributions run tests on their plugins).
> > - Startup time is not negatively affected.
> > - Build/prepare time is not negatively affected.
> >
> > Is that fair?
> >
>
> Absolutely totally fair. Those are valid concerns.
>
> I tried the core plugins and they all work and all pass mobile-spec. I know
> you guys (ab)use require('./Module') a lot in chrome plugins but they
> should still all pass. I actually would be curious to know the results.
>

I just checked our code, and the only place I see that construct in plugins
is in org.chromium.runtime -- it uses "require('./CryptoJS-sha256')" to
access the CryptoJS library, without having to make that a separate plugin.
Is there a better way that we should be doing that?


> Startup should be faster because it is not XHRing a different file to get a
> list of plugins to <script> inject them. I actually didn't benchmark this
> but it would be interesting.
>

This is a win -- I remember that we were all pretty excited about not
having all of the code baked in to cordova.js when 3.0 came out, but people
have complained that the result is slower startup times, because of the
extra XHRs.

Build time is slowed down by a few ms (re-read the first email of this
> thread).
>

And, of course, that's absolutely dwarfed by every other measurable factor.

I think this is a good step forward (so long as it's all working) -- If we
just switch the default, then people still have the ability to build
without it, right?

I just wonder whether semver requires us to bump the major version of
anything because of this (and if so, then what? CLI? JS? Lib? Everything?)
or whether a "no breaking changes, no new features" change can count as a
patch version increment.


> >
> > -Michal
> >
> > On Thu, Dec 11, 2014 at 2:38 PM, Andrew Grieve <agrieve@chromium.org>
> > wrote:
> >
> > > I'd really like to get it fully spelled out *why* browserify is the
> right
> > > tool for this. Some thoughts below:
> > >
> > > On Wed, Dec 10, 2014 at 8:35 PM, Brian LeRoux <b@brian.io> wrote:
> > >
> > > > we should move browserify to main and drop that insane concat code
> > > >
> > > > its not heavyweight at all. it creates a hash in iife with deps
> mapped
> > > > in…as to why dep mgmt is better than concatenating…I don't think we
> > need
> > > to
> > > > waste our time talking about that!
> > >
> > >
> > >
> > > What makes the concat code insane?
> > >  - It's easy to understand and make changes to
> > >  - It seems to work fine and is quite stable
> > >  - It has no dependencies (it's 52kb within cordova-js/tasks/lib)
> > >
> > > Browserify seems a bit scary to me:
> > > - It's 25MB and has a tonne of dependencies
> > > - Do we really need all of that functionality?
> > >
> > > I think it would actually help a lot to talk about dependency
> management.
> > > - Right now cordova-js creates a module for every file in src/
> > > - Plugins list each module explicitly in their plugin.xml
> > > With browserify, are you envisioning that this model change at all?
> > >
> > >
> > > Why change anything?
> > > - cordova-lib should concat plugin modules with cordova.js on prepare
> > (this
> > > is the motivation stated in the JIRA. totally on board with this!)
> > > What else would I like to see changed?
> > > - exec.js (and any other platform-specific modules) should move to live
> > in
> > > platform repos
> > >   - This will make code changes that affect both much easier
> > > - Platforms should depend on cordova-js via package.json and snapshot
> > > generation should happen in bin/create as well as plugin add/rm
> > >   - This will remove the (somewhat) annoying step of snapshotting
> > >   - This will allow cordova-js to be released as it's own artifact
> > >     - Good because we can do things like add Promise polyfill and not
> > need
> > > to release all platforms to pick it up
> > >     - A bit annoying since it's one more thing to release, but likely
> > won't
> > > get released often.
> > >
> > >
> > > Please don't look at my email as trying to derail the work Anis did. I
> > > believe most of it is necessary to accomplish these goals whether we
> use
> > > Browserify, our own, or some other concattenator. I just question
> whether
> > > 25MB is too much complexity for what we need / want.
> > >
> > >
> > >
> > >
> > > On Thu, Dec 11, 2014 at 2:10 PM, Joe Bowser <bowserj@gmail.com> wrote:
> > >
> > > > This should be a major, but yeah, I'm fine with making this the
> > default.
> > > >
> > > > On Thu, Dec 11, 2014, 11:08 AM Brian LeRoux <b@brian.io> wrote:
> > > >
> > > > > so I think this has baked long enough! lets make it the default and
> > > suss
> > > > > the bugs.
> > > > >
> > > > > On Thu, Jul 10, 2014 at 3:36 AM, Ally Ogilvie <aogilvie@wizcorp.jp
> >
> > > > wrote:
> > > > >
> > > > > > Ace, look forward to browser/web as platform. Combined Web and
> > Native
> > > > API
> > > > > > plugin for Cordova! Yay!
> > > > > > Thanks for the clarification Michal.
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Tue, Jul 8, 2014 at 6:07 AM, Anis KADRI <anis.kadri@gmail.com
> >
> > > > wrote:
> > > > > >
> > > > > > > Right, browserify is just a way to package cordova.js that
> should
> > > > make
> > > > > it
> > > > > > > easier to add the browser as a platform so might not be
> directly
> > > > > related
> > > > > > > but is somewhat related :-}
> > > > > > >
> > > > > > >
> > > > > > > On Mon, Jul 7, 2014 at 4:45 PM, Michal Mocny <
> > mmocny@chromium.org>
> > > > > > wrote:
> > > > > > >
> > > > > > > > I think Ally may be confusing what this does?: This
> browserify
> > > work
> > > > > is
> > > > > > > not
> > > > > > > > a way to get cordova to run in a desktop browser,
its just a
> > > means
> > > > > for
> > > > > > > > packaging cordova.js.  I think there should be no
visible
> > change
> > > to
> > > > > end
> > > > > > > > users.
> > > > > > > >
> > > > > > > > There is a separate effort to target the browser as
a
> platform,
> > > and
> > > > > it
> > > > > > > may
> > > > > > > > be true that the browserify work helps with that,
but its not
> > > > > directly
> > > > > > > > related.
> > > > > > > >
> > > > > > > >
> > > > > > > > On Mon, Jul 7, 2014 at 3:58 PM, Anis KADRI <
> > anis.kadri@gmail.com
> > > >
> > > > > > wrote:
> > > > > > > >
> > > > > > > > > On Sun, Jun 29, 2014 at 9:54 PM, Ally Ogilvie
<
> > > > aogilvie@wizcorp.jp
> > > > > >
> > > > > > > > wrote:
> > > > > > > > >
> > > > > > > > > > Anis that is really sweet.
> > > > > > > > > > If this hits CLI, plugin.xml will have sections
for
> plugins
> > > to
> > > > do
> > > > > > web
> > > > > > > > > > actions?
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Not sure I understand what you mean by web actions.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > > TBH.. i've always wanted a cordova platform
add web...
> but
> > > i'd
> > > > be
> > > > > > > happy
> > > > > > > > > > enough with a prepare for browser only mode.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > `cordova platform add web` is part of the plan.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > Seeing a lot of use cases (e.g. Facebook
plugin etc.)
> where
> > > > there
> > > > > > are
> > > > > > > > JS,
> > > > > > > > > > iOS & Android SDKs and a Cordova Plugin
wants to support
> > the
> > > > same
> > > > > > API
> > > > > > > > on
> > > > > > > > > > all platforms.
> > > > > > > > > >
> > > > > > > > >
> > > > > > > > > Didn't think of that but it is applicable indeed.
> > > > > > > > >
> > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > On Sat, Jun 21, 2014 at 9:26 AM, Anis KADRI
<
> > > > > anis.kadri@gmail.com>
> > > > > > > > > wrote:
> > > > > > > > > >
> > > > > > > > > > > Ok cool. I can look at adding a --browserify
option for
> > run
> > > > and
> > > > > > > > > prepare.
> > > > > > > > > > I
> > > > > > > > > > > logged an issue for it [1]
> > > > > > > > > > >
> > > > > > > > > > > [1] https://issues.apache.org/jira/browse/CB-7001
> > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > > > On Thu, Jun 19, 2014 at 5:57 PM, Andrew
Grieve <
> > > > > > > agrieve@chromium.org
> > > > > > > > >
> > > > > > > > > > > wrote:
> > > > > > > > > > >
> > > > > > > > > > > > Thanks Anis!
> > > > > > > > > > > >
> > > > > > > > > > > > Tougher for CLI since it's actually
the prepare step
> > that
> > > > > > creates
> > > > > > > > > > > > cordova_plugins.js, but longer
term (medium term?) I
> > > don't
> > > > > see
> > > > > > > why
> > > > > > > > we
> > > > > > > > > > > > shouldn't just turn it on always
anyways.
> > > > > > > > > > > >
> > > > > > > > > > > > So... Maybe cordova prepare --browserify?
> > > > > > > > > > > > Build prepares first, so will
also need: cordova run
> > > > android
> > > > > > > > > > --browserify
> > > > > > > > > > > >
> > > > > > > > > > > > I haven't looked at it yet. Google
IO is next week
> and
> > > it's
> > > > > > been
> > > > > > > > > > > consuming
> > > > > > > > > > > > most of our time the last few
weeks. Will definitely
> > play
> > > > > with
> > > > > > it
> > > > > > > > > next
> > > > > > > > > > > next
> > > > > > > > > > > > week though!
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > > > On Thu, Jun 19, 2014 at 6:28 PM,
Anis KADRI <
> > > > > > > anis.kadri@gmail.com>
> > > > > > > > > > > wrote:
> > > > > > > > > > > >
> > > > > > > > > > > >> Sorry. I forgot you asked
the question. There was no
> > > issue
> > > > > but
> > > > > > > > there
> > > > > > > > > > is
> > > > > > > > > > > >> one now.
> > > > > > > > > > > >> https://issues.apache.org/jira/browse/CB-6990
> > > > > > > > > > > >>
> > > > > > > > > > > >> This "feature" is plugman
only for now. How
> important
> > is
> > > > it
> > > > > to
> > > > > > > > wire
> > > > > > > > > it
> > > > > > > > > > > >> to CLI ? Have  you guys had
time to test it out yet
> ?
> > > How
> > > > > > would
> > > > > > > it
> > > > > > > > > > > >> work with CLI ? Add another
flag such as "cordova
> > plugin
> > > > add
> > > > > > > > > > > >> --browserify" ?
> > > > > > > > > > > >>
> > > > > > > > > > > >> On Thu, Jun 19, 2014 at 9:28
AM, Andrew Grieve <
> > > > > > > > > agrieve@chromium.org>
> > > > > > > > > > > >> wrote:
> > > > > > > > > > > >> > bump
> > > > > > > > > > > >> >
> > > > > > > > > > > >> >
> > > > > > > > > > > >> > On Mon, Jun 16, 2014
at 12:51 PM, Andrew Grieve <
> > > > > > > > > > agrieve@chromium.org
> > > > > > > > > > > >
> > > > > > > > > > > >> > wrote:
> > > > > > > > > > > >> >>
> > > > > > > > > > > >> >> Cool, yes! Thanks
for the update!
> > > > > > > > > > > >> >>
> > > > > > > > > > > >> >> Is there a JIRA for
this? Was asked in
> > > > > > > > > > > >> >> https://issues.apache.org/jira/browse/CB-5671.
> > > > > > > > > > > >> >>
> > > > > > > > > > > >> >>
> > > > > > > > > > > >> >>
> > > > > > > > > > > >> >>
> > > > > > > > > > > >> >> On Mon, Jun 16, 2014
at 10:21 AM, Michal Mocny <
> > > > > > > > > > mmocny@chromium.org>
> > > > > > > > > > > >> >> wrote:
> > > > > > > > > > > >> >>>
> > > > > > > > > > > >> >>> Awesome Anis.
> > > > > > > > > > > >> >>>
> > > > > > > > > > > >> >>> Will gladly take
a look at this later today.
> Just
> > > > > wanted
> > > > > > to
> > > > > > > > > send
> > > > > > > > > > a
> > > > > > > > > > > >> quick
> > > > > > > > > > > >> >>> thanks for landing
this this way, and for the
> > useful
> > > > > > report.
> > > > > > > > > > > >> >>>
> > > > > > > > > > > >> >>> -Michal
> > > > > > > > > > > >> >>>
> > > > > > > > > > > >> >>>
> > > > > > > > > > > >> >>> On Fri, Jun 13,
2014 at 7:55 PM, Anis KADRI <
> > > > > > > > > anis.kadri@gmail.com
> > > > > > > > > > >
> > > > > > > > > > > >> wrote:
> > > > > > > > > > > >> >>>
> > > > > > > > > > > >> >>> > Yo,
> > > > > > > > > > > >> >>> >
> > > > > > > > > > > >> >>> > Just wanted
to let everyone know that I added
> > > > > browserify
> > > > > > > > > support
> > > > > > > > > > > to
> > > > > > > > > > > >> >>> > plugman
(behind a flag for now). CLI is not
> > hooked
> > > > to
> > > > > > this
> > > > > > > > > yet.
> > > > > > > > > > > Here
> > > > > > > > > > > >> >>> > is how it
works:
> > > > > > > > > > > >> >>> >
> > > > > > > > > > > >> >>> > plugman
install --browserify --plugin [PLUGIN]
> > > > > > --platform
> > > > > > > > > > > [PLATFORM]
> > > > > > > > > > > >> >>> > --project
[PROJECT_PATH]
> > > > > > > > > > > >> >>> >
> > > > > > > > > > > >> >>> > will generate
a browserify version of
> > cordova.js.
> > > > > > Plugins
> > > > > > > > and
> > > > > > > > > > > >> >>> > everything
is bundled in. This version passes
> > > > > > mobile-spec
> > > > > > > on
> > > > > > > > > iOS
> > > > > > > > > > > and
> > > > > > > > > > > >> >>> > Android.
I am not yet setup to test other
> > > platforms.
> > > > > > > > > > > >> >>> >
> > > > > > > > > > > >> >>> > plugman
install --plugin [PLUGIN] --platform
> > > > > [PLATFORM]
> > > > > > > > > > --project
> > > > > > > > > > > >> >>> > [PROJECT_PATH]
> > > > > > > > > > > >> >>> >
> > > > > > > > > > > >> >>> > Will continue
to generate cordova.js the way
> it
> > > used
> > > > > to.
> > > > > > > > > > > >> >>> >
> > > > > > > > > > > >> >>> > Because
some of you really care about
> benchmarks
> > > > here
> > > > > is
> > > > > > > > some
> > > > > > > > > > > >> >>> > comparison
for dependencies-plugin install:
> > > > > > > > > > > >> >>> >
> > > > > > > > > > > >> >>> > No browserify:
> > > > > > > > > > > >> >>> >
> > > > > > > > > > > >> >>> > real 0m9.546s
> > > > > > > > > > > >> >>> > user 0m4.673s
> > > > > > > > > > > >> >>> > sys 0m0.692s
> > > > > > > > > > > >> >>> >
> > > > > > > > > > > >> >>> > Browserify:
> > > > > > > > > > > >> >>> > real 0m9.861s
> > > > > > > > > > > >> >>> > user 0m4.759s
> > > > > > > > > > > >> >>> > sys 0m0.648s
> > > > > > > > > > > >> >>> >
> > > > > > > > > > > >> >>> > All cordova-lib
tests are passing so I am
> > assuming
> > > > > this
> > > > > > > has
> > > > > > > > > > > minimal
> > > > > > > > > > > >> >>> > impact but
LET ME KNOW otherwise.
> > > > > > > > > > > >> >>> >
> > > > > > > > > > > >> >>> > Anis
> > > > > > > > > > > >> >>> >
> > > > > > > > > > > >> >>
> > > > > > > > > > > >> >>
> > > > > > > > > > > >> >
> > > > > > > > > > > >>
> > > > > > > > > > > >
> > > > > > > > > > > >
> > > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > >
> > > > > > > > > > --
> > > > > > > > > > <http://www.wizcorp.jp/>Ally Ogilvie
> > > > > > > > > > Lead Developer - MobDev. | Wizcorp Inc.
<
> > > > http://www.wizcorp.jp/>
> > > > > > > > > > ------------------------------
> > > > > > > > > > TECH . GAMING . OPEN-SOURCE WIZARDS+ 81
(0)3-4550-1448 |
> > > > Website
> > > > > > > > > > <http://www.wizcorp.jp/> | Twitter
<
> > > > https://twitter.com/Wizcorp>
> > > > > |
> > > > > > > > > > Facebook
> > > > > > > > > > <http://www.facebook.com/Wizcorp>
| LinkedIn
> > > > > > > > > > <http://www.linkedin.com/company/wizcorp>
> > > > > > > > > >
> > > > > > > > >
> > > > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > --
> > > > > > <http://www.wizcorp.jp/>Ally Ogilvie
> > > > > > Lead Developer - MobDev. | Wizcorp Inc. <http://www.wizcorp.jp/>
> > > > > > ------------------------------
> > > > > > TECH . GAMING . OPEN-SOURCE WIZARDS+ 81 (0)3-4550-1448 | Website
> > > > > > <http://www.wizcorp.jp/> | Twitter <https://twitter.com/Wizcorp>
> |
> > > > > > Facebook
> > > > > > <http://www.facebook.com/Wizcorp> | LinkedIn
> > > > > > <http://www.linkedin.com/company/wizcorp>
> > > > > >
> > > > >
> > > >
> > >
> >
>

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message