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

PlatformApi definition and documentation. #9

Closed
wants to merge 8 commits into from

Conversation

vladimir-kotikov
Copy link
Member

This proposal describes a high-level classes which describes Cordova project and Cordova platform as a strictly typed APIs:

  • ProjectApi - a high-level abstraction on top of cordova project, exposes project's properties and methods for project management.
  • PlatformApi class - further development of PlatformProject (see PlatformProject and platform specific code refactoring #4) - abstraction for single particular platform.

The aim of this document is to discuss and finalize API details and provide a reference for implementing ProjectApi and PlatformApi for new platforms or updating the old ones to new model.

To make the transition to new API smoother, the cordova-lib needs to be updated to expose the same class, which will work as a polyfill in case if platform doesn't provides its own PlatformApi.

There also a potential problem of code duplication, since the PlatformApi needs for several classes, which currently being stored and exposed by cordova-lib: ConfigParser, PluginInfo, etc. The one of solutions of this problem - is to factor out the common classes and routines to shared module (proposed name - cordova-common), which then will be released separately. This approach is discussed in cordova-lib/#235.

The transition steps might be the following:

  • Implement a PlatformApi polyfill for cordova-lib to hide the existing implementation and provide a fallback implementation for platforms, that doesn't expose own PlatformApi. See https://github.com/MSOpenTech/cordova-lib/pull/2
  • Factor out shared logic to cordova-common module and publish it separately.
  • Implement and expose PlatformApi classes for major platforms.

Still in progress:

  • Add description/definition for ProjectInfo/PlatformInfo classes

More links:

@vladimir-kotikov
Copy link
Member Author

As and addition to this proposal, to make exception handling more accurate in high-level API, we may want to have a number of different error classes, or introduce an CordovaError.code field and a set of corresponding constants, such as CordovaError.PLATFORM_API_NOT_IMPLEMENTED_ERROR, CordovaError.PLATFORM_API_ARGUMENT_ERROR, ...

@vladimir-kotikov
Copy link
Member Author

@purplecabbage, @stevengill, Please take a look in a spare time.

@stevengill
Copy link
Contributor

This looks great! Awesome job Vladimir.

Do you plan to integrate https://github.com/apache/cordova-lib/tree/master/cordova-lib/src/cordova/metadata and https://github.com/apache/cordova-lib/tree/master/cordova-lib/src/plugman/platforms replacements into this. Would some of these files go into cordova-common?

Thoughts on having a updatePlatform and updatePlugin api?

I will have to play around with it more as we implement it. I'm definitely down to help with this as I have wanted cordova to work better with non cli node projects for a while.

Not sure if this is relevant to this discussion, but I noticed plugin dependencies get handled a layer above this. So cordova-lib would handle dependencies for cordova-cli. I'm curious about how this will be handled in a node project with plugins + platforms defined in package.json under dependency. addPlugin function would read plugin.xml files and install based on that, but would presumably ignore the dependency tag and not install plugins. Users would have to install them manually. Worth discussing this workflow a bit more I think.

@vladimir-kotikov
Copy link
Member Author

Thanks, Steve.

Do you plan to integrate...

Yes, the logic from cordova/metadata and plugman/platforms will be reused in a PlatformApi polyfill in cordova-lib. Regarding moving it to cordova-common - It might make sense in terms of deduping the code, but i would vote for just copying it into platforms. There is not too much code for each particular platform to dedupe, instead we'll be able to modify platform's logic easily and freely and will not depend on legacy code.

Thoughts on having a updatePlatform and updatePlugin api?

Definitely will add it.

Regarding managing dependencies - if we decide to do this in platform, we'll need to make platform responsible for fetching plugins - thing I would like to avoid. It might make sense to factor dependecy resolver into cordova-common as a separate class. Thoughts?

Vladimir Kotikov added 3 commits June 29, 2015 17:39
  - Adds updatePlatform static method,
  - Makes getPlatformInfo method non-static,
  - Updates removePlugin method to accept PluginInfo instance as input
  parameter instead of plugin id.
@vladimir-kotikov
Copy link
Member Author

Updated PlatformApi slightly according to notes on this PR and added description for CordovaProject class which abstracts project-related information.

@vladimir-kotikov
Copy link
Member Author

Thanks @nikhilkh, addressed your notes in 15fb001. Also finalized and added a ProjectApi reference.

/**
* Gets a PlatformInfo object, that represents the platform structure.
*
* @return {PlatformInfo} A structure that contains the description of
Copy link
Contributor

Choose a reason for hiding this comment

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

Where is the definition for PlatformInfo? Should this be called CordovaPlatform or PlatformApi be called CordovaPlatform to make the naming consistent with CordovaProject

Copy link
Member Author

Choose a reason for hiding this comment

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

Added definition in 4992734 and updated the name. However, we might need an additional discussion on naming classes.

@vladimir-kotikov
Copy link
Member Author

Closing this in favor of #12

*
* @type {String}
*/
cordovaJs: path.join('bin', 'template', 'www', 'cordova.js'),
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we be exposing this file ? I think we should expose the one that gets generated by 'prepare' and is in the 'www' folder.

@vladimir-kotikov
Copy link
Member Author

Can't remember exactly, but this looks like the part of compatibility stuff. The cordova-js source path is required somewhere inside cordova-lib. But in general, yes, you're right, this is not really required to be exposed.

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.

4 participants