You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@cordova.apache.org by vladimir-kotikov <gi...@git.apache.org> on 2015/07/31 16:20:31 UTC
[GitHub] cordova-lib pull request: CB-9436 Removes `require-tr` bundle tran...
GitHub user vladimir-kotikov opened a pull request:
https://github.com/apache/cordova-lib/pull/275
CB-9436 Removes `require-tr` bundle transformation
This PR replaces excess `require-tr` browserify transform (- 2 deps and lot of code) with `aliasify`.
It also solves two more problems from [Browserify Master Issue](https://issues.apache.org/jira/browse/CB-8801):
* browserify and mobilespec (see https://issues.apache.org/jira/browse/CB-8802) - because all bundled modules now exposed outside of bundle
* excess plugins' dirs in `www` folder (see https://issues.apache.org/jira/browse/CB-9038)
NOTE: This PR depends on apache/cordova-js#255.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/MSOpenTech/cordova-lib CB-9436
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/cordova-lib/pull/275.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #275
----
commit 510516135ef41b735b2e610c1b8d95fc77bd571e
Author: Vladimir Kotikov <v-...@microsoft.com>
Date: 2015-07-31T11:20:35Z
CB-9436 Removes `require-tr` bundle transformation
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-9436 Removes `require-tr` bundle tran...
Posted by vladimir-kotikov <gi...@git.apache.org>.
Github user vladimir-kotikov commented on the pull request:
https://github.com/apache/cordova-lib/pull/275#issuecomment-127782310
> Wondering why you had to add require('codrdova/init') to the bundle? MSOpenTech@5105161#diff-28a16640f3807e6ee2300ea7ce8c2f1dR196. How was that getting added previously?
Previously this was being added by `cordova-js/tasks/lib/bundle-browserify.js` as `cordova/bootstrap` module. The problem is when you adding plugin modules into bundle after bootstrap is added, they only get loaded _after_ cordova initialization and `deviceready` event. The only way i found to fix this, is to add `bootstrap` as the last file in the bundle.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-9436 Removes `require-tr` bundle tran...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/cordova-lib/pull/275
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-9436 Removes `require-tr` bundle tran...
Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on the pull request:
https://github.com/apache/cordova-lib/pull/275#issuecomment-127799658
Make sense. Thanks! Lets merge it in!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-9436 Removes `require-tr` bundle tran...
Posted by stevengill <gi...@git.apache.org>.
Github user stevengill commented on the pull request:
https://github.com/apache/cordova-lib/pull/275#issuecomment-127452233
Vladimir! This is a much needed refactor and it looks great! Thank you! The previous spaghetti code was becoming hard to manage and understand.
Things I like:
* Using `cordova_plugins.js` in the browserify build and removing `cordova_requires`
* Adding the module name field in `cordova_plugins.js` for each module.
* Using browserify expose to map `cordova/plugin_list` to `cordova_plugins.js`. (Didn't even know about expose)
* Great find with aliasify! Much cleaner way to handle the bad legacy requires in plugins compared to using requiresTr.
Quick Question without digging to deep:
* Wondering why you had to add require('codrdova/init') to the bundle? https://github.com/MSOpenTech/cordova-lib/commit/510516135ef41b735b2e610c1b8d95fc77bd571e#diff-28a16640f3807e6ee2300ea7ce8c2f1dR196. How was that getting added previously?
I think you fixed all of the remaining bugs with your changes. @surajpindoria will continue to test on all of the devices we have here to see if we can find bugs.
I am going to dive deeper into your cordova.js changes now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org
[GitHub] cordova-lib pull request: CB-9436 Removes `require-tr` bundle tran...
Posted by nikhilkh <gi...@git.apache.org>.
Github user nikhilkh commented on a diff in the pull request:
https://github.com/apache/cordova-lib/pull/275#discussion_r36097029
--- Diff: cordova-lib/src/plugman/prepare-browserify.js ---
@@ -21,7 +21,7 @@
var platform_modules = require('../platforms/platforms'),
path = require('path'),
- through = require('through2'),
+ aliasify = require('aliasify'),
--- End diff --
Should the 'through2' dependency be removed from package.json?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@cordova.apache.org
For additional commands, e-mail: dev-help@cordova.apache.org