Differences between plugins

[dcousens]

Hi @akofman,

I'm asking here because I was hoping to find out why there exists two plugins with near-identical features.

https://github.com/bitjson/cordova-plugin-swift-support and this plugin.
Ping @bitjson? Would you be willing to just submit a PR for any changes you made?

Is this plugin still maintained?

[dcousens]

In #16 (comment), @bitjson mentioned he was happy to submit the changes up stream, @akofman what were your thoughts?

[akofman]

Hello @dcousens,

This plugin is still maintained of course !
I don't personally know @bitjson but I already told him that PR are very welcomed.
Looking at its fork, I don't think it adds any new feature now and it is not up-to-date anymore. It looks more like an opinionated version with different licence and different commit convention.
I agree it's quite confusing to have two different versions from npm :/

[dcousens]

One difference I noticed was node_modules.
Why is node_modules committed in this version?

[akofman]

Sure, it's because this plugin has to keep compatibility with still used legacy versions of Cordova (<5). In these versions, the node-xcode lib was an older one with different semantics. So we need to use a bundledDependencies of this lib to be sure to use the same version regardless of the Cordova version.

I hope it helps.

[akofman]

And also because Cordova is not able to install an npm dependency from its plugin mechanism.

[dcousens]

@akofman is there no way to rectify that? That is a massive security issue and it would block me from using this module outright 😢

[akofman]

ah ? Why is that a security issue ?

[dcousens]

@akofman because I have to trust that the bundled node_modules has not been modified, and that is a much harder task to do when you have multiple copies sitting in various node_modules compared to pinning a single version.

[akofman]

The bundled node_module will always stay the same cause it is bundled. I mean It's not like a classic dependency; If I don't push anything your installed version will always use the same version of this one. The Cordova plugin mechanism won't try to update it, it's not like npm. It is really like these deps are part of this plugin as any other source file. Did I understand well your issue ?

[akofman]

https://github.com/akofman/cordova-plugin-add-swift-support/blob/master/package.json#L34-L36

[dcousens]

I'm sorry, but I'm still struggling to understand why you can't just specify the xcode version in dependencies in package.json and that be good enough.

I have several projects that use the version by @bitjson (as a sub-dependency) and it works fine?
Since I don't need to worry about legacy, the security concern doesn't seem worth it... 😕

Apologies if I'm misunderstanding something, I don't mean to be a pain, I just wanted to establish my concerns now to avoid switching libraries again in future.

[akofman]

No problem, I'll try to be clearer :)
I want to keep this plugin compatible with older version of Cordova and I want to keep the possibility to install it from the npm repo and from the github repo.

As an example if you try to install the @bitjson plugin from the github repo you will have an error like:

$ cordova plugin add https://github.com/bitjson/cordova-plugin-swift-support.git
Fetching plugin "https://github.com/bitjson/cordova-plugin-swift-support.git" via git clone
Repository "https://github.com/bitjson/cordova-plugin-swift-support.git" checked out to git ref "master".
Installing "cordova-plugin-swift-support" for ios
Error: Cannot find module 'xcode'
    at Function.Module._resolveFilename (module.js:440:15)
    at Function.Module._load (module.js:388:25)
    at Module.require (module.js:468:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/Users/Alexis/Programming/cordova-test/plugins/cordova-plugin-swift-support/src/add-swift-support.js:20:13)
    at Module._compile (module.js:541:32)
    at Object.Module._extensions..js (module.js:550:10)
    at Module.load (module.js:458:32)
    at tryModuleLoad (module.js:417:12)
    at Function.Module._load (module.js:409:3)

The @bitjson version works for you because you are using an up-to-date version of Cordova and because you install it from the npm repo. This is one usecase among many others.

That's why I have to use a bundledependency and also to push the node_modules folder but I don't think it's a security issue. If you install two plugins with differents xcode bundledependency you won't have any security issue because of version overriding. Your plugins are installed in the plugins folder of your project with their own node_modules folder.

Did you try to reproduce the security issue you're talking about ?

Anyway, you're free to use the version you want if it works for you but you just have to know that this one is still maintained and will have some updates in the future for sure.
And you don't have to apologize, it's cool to know this project is used or just looked :)

[bitjson]

Looks like this has been covered now, but just to clarify – the only changes in my fork are:

• no committed node_modules
• major version follows the supported Swift major version (currently 3.1.0 for Swift 3)
• linting with xo and some cosmetic changes
• auto-generated changelog
• MIT license

Really just opinionated project changes 😄

@akofman if you want anything from above, I'm happy to help merge it.

[dcousens]

Thanks @bitjson and @akofman for the explanations 👍

[akofman]

@bitjson oh sorry if I offended you. What I meant is that the actual version of this plugin already supports Swift 3 and as I explained before, I don't want to remove the node_modules folder for compatibility reasons. So the remaining additions are xo linting, auto changelog and the MIT licence which are not really issues but personal choices. I'm more confortable with eslint, manual changelog and the Apache licence. Thanks for your suggestions and I'm available to talk about it if needed.

[dcousens]

@akofman would you maybe be willing to maintain a backport for legacy reasons, then you could ditch the node_modules in the latest version?

[akofman]

Unfortunately It would still be an issue in case of install from the github repo, legacy version or not.

[akofman]

Ok @dcousens, I finally delivered this plugin from a webpack bundle so that it doesn't contain any dependency neither a node_modules folder. And it will permit me to have a better code structure for the next steps ;)

[bitjson]

@akofman I was agreeing with you 😄

I assumed you were happy with the current setup. Thanks for explaining your reasoning on the node_modules too, I think this thread has been really helpful.