-
Notifications
You must be signed in to change notification settings - Fork 28
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
Better integrate with module bundlers #73
base: master
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the proposal @remcohaszing!
Some really interesting points of discussion in here. I like the goals of getting cordova to play nice with all of the existing build tools out there currently. It aligns with our goal to modernize cordova I'd say.
cordova.js
definitely needs to be rethought-out and reimplemented. The browserify rewrites are complicated. I suspect part of the refactor will be updating some plugins as well.
I've added some notes to the proposal. I look forward to seeing this discussion progress. I'll chime in more later this week.
|
||
Make Cordova integrate better with a modern JavaScript setup. | ||
|
||
Basically it comes down to that Cordova should allow using node modules. This allows better integration with tooling such as: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes! This would be sweet
- Get rid of `www/`. | ||
- Get rid of `plugins/`. | ||
|
||
### 1. Load plugins from the `node_modules` directory, not `plugins` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
An issue for this has already been filed https://issues.apache.org/jira/browse/CB-13059. Also it is proposed in the cordova@8 proposal #72
|
||
This also allows plugin creators to use npm dependencies for both hooks and browser code the way this would be done for other packages. | ||
|
||
### 2. Let a developer require Cordova using its own build tools. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if i'm understanding this correctly, you are suggesting we use html imports to do plugin loading.
Few concerns:
-
I think this would be a tough sell for plugins. Pretty much every plugin would have to change.
-
Our goal is to mimic web apis. Camera is a bad example because it isn't based on a standard. But a lot of the current work on our plugin audit is about deprecating non standard apis (We are keeping camera don't worry) and updating apis of plugins where the standard has changed since we implemented it originally.
-
imports aren't supported everywhere i think. Would that force users to use a build tool?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So if i'm understanding this correctly, you are suggesting we use html imports to do plugin loading.
I'm not suggesting to use html imports, but I'm suggesting to require a build tool. These tools are already specialized at handling node modules, so Cordova doesn't have to handle this differently. Anywhere where I write "import
", it might just as well say "require
".
Few concerns:
- I think this would be a tough sell for plugins. Pretty much every plugin would have to change.
One implementation of such a build hook could mimic the old behaviour for a while.
- Copy the contents of
wwww
into the platform www directory. - Use browserify (or webpack) to detect all Cordova plugins and the Cordova platform directory, bundle those and write this into
cordova.js
in the platform www directory. - For backwards compatibility, this could map
cordova
imports tocordova-js
andcordova/*
tocordova-js/*
.
This could be included by default in cordova-app-hello-world
.
I would personally prefer a hook that would bundle and minify everything, including my own code, my own dependnecies and Cordova code. This means I would have to write my own hook for this.
- Our goal is to mimic web apis. Camera is a bad example because it isn't based on a standard. But a lot of the current work on our plugin audit is about deprecating non standard apis (We are keeping camera don't worry) and updating apis of plugins where the standard has changed since we implemented it originally.
Mimicking (or polyfilling) web APIs doesn't require any special handling from Cordova. Just assign values to the window
object.
- imports aren't supported everywhere i think. Would that force users to use a build tool?
This would force users to include a build hook. This is likely to be based on a build tool.
Plugins would make calls to `cordova-js/exec` (replacing `cordova/exec`) independant of the platform. If a developer wishes to use an outdated plugin, it's the developer's responsibility to alias `cordova/*` calls to `cordova-js/*`. | ||
|
||
|
||
### 4. Dependency management |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We looked into peer dependencies at one point. We found out they were getting deprecated. https://docs.npmjs.com/files/package.json#peerdependencies
They don't auto install peer deps anymore. They just omit warnings to the user.
The reason why cordova needs to manage plugin deps is because we can't allow a plugin to have two different versions installed in one project.
For example, say file-transfer plugin requires file version 3. And say media plugin requires file version 4. This would and should cause an issue. Since the native implementations of file would clash on the native side (duplication errors).
npm has never given us a way to manage this unique case so we have always had to have our own level of dep management
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We looked into peer dependencies at one point. We found out they were getting deprecated. https://docs.npmjs.com/files/package.json#peerdependencies
They don't auto install peer deps anymore. They just omit warnings to the user.
The reason why cordova needs to manage plugin deps is because we can't allow a plugin to have two different versions installed in one project.
For example, say file-transfer plugin requires file version 3. And say media plugin requires file version 4. This would and should cause an issue. Since the native implementations of file would clash on the native side (duplication errors).
npm has never given us a way to manage this unique case so we have always had to have our own level of dep management
Their behaviour has changed between npm versions 2 and 3, but it's not deprecated as far as I know.
I believe giving a warning is the way npm has given to manage flat dependencies.
- ESLint recommends it for shareable configs: http://eslint.org/docs/developer-guide/shareable-configs#publishing-a-shareable-config
- React libraries add
react
as a peer dependency. - Angular even uses peer dependencies for depending on
@angular/*
packages.
I don't think the situation for Cordova is any different. How npm should handle these missing dependencies is out of scope for Cordova. Users shouldn't be surprised if their builds fail when using mismatched versions.
Also if plugins depend on other plugins, this can be handled by specifying them as `peerDependencies`. | ||
|
||
|
||
### 5. Fix up the browserify setup |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the browserify code is confusing. With the refactoring of cordova.js we could potentially remove this and allow our devs to use one of the various build step tools in their own setup
"main": "www/Camera.js" | ||
``` | ||
|
||
### 3. Use [cordova-js] as a proxy to the platform |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cordova-js definitely needs to be rethought out. This is an interesting idea I'd like to hear more discussion about.
As someone who uses Cordova to build web apps, I'm pretty opposed to anything that requires my app code to know that Cordova exists. I'm building apps for the web, Cordova is just a packaging tool and polyfill loader (in the form of plugins). My app should not know or care that it's running in Cordova. Currently Cordova requires the app to include Plugins can already require node modules using npm via their package.json files if Cordova is run with the |
@dpogue |
I'd love to see better integration with module bundlers, such as webpack.
I think I figured out the global steps of what needs to be done for this.
I might be missing a lot of details, because I'm not that familiar with Cordova internals.