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

Plugin Import updates #3305

Merged
merged 8 commits into from
Jul 23, 2020
Merged

Plugin Import updates #3305

merged 8 commits into from
Jul 23, 2020

Conversation

imhoffd
Copy link
Contributor

@imhoffd imhoffd commented Jul 21, 2020

This is an implementation of the plugin import proposal. To achieve consistent imports in each platform, I recommend we move the Plugins proxy from user code to plugin author code.

registerPlugin should be used to register implementations of plugins in each platform
registerWebPlugin will continue to work so people can continue using the plugin proxy and the web implementation (we can avoid a breaking change)

TODO

  • add a compat.ts for legacy (PluginRegistry, etc.)

closes: #3306

@imhoffd imhoffd marked this pull request as ready for review July 21, 2020 21:27
@imhoffd imhoffd requested review from mlynch and jcesarmobile July 21, 2020 21:47
Copy link
Contributor

@mlynch mlynch left a comment

Choose a reason for hiding this comment

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

First pass looks good, where do the existing web plugins end up getting registered now? Or will that come w/ them being split out into separate packages?

@imhoffd
Copy link
Contributor Author

imhoffd commented Jul 21, 2020

registerWebPlugin got moved here: https://github.com/ionic-team/capacitor/pull/3305/files#diff-1cb8bd56af1537f63fe48eb4fd61aad0R85-R104

Existing plugins should continue working, with a console warning. We'll have to add an upgrade guide for plugins to switch to the new registerPlugin method so that the user can use the new imports across all platforms.

@mlynch
Copy link
Contributor

mlynch commented Jul 21, 2020

I saw that but wasn't sure where the actual registration was happening anymore now that this line is gone: https://github.com/ionic-team/capacitor/pull/3305/files#diff-8aa3e4905a661a3f822d1f6cdf593c15L22

@imhoffd
Copy link
Contributor Author

imhoffd commented Jul 21, 2020

Oops, updated!

@mlynch
Copy link
Contributor

mlynch commented Jul 22, 2020

I'm still missing where the actual merge/register is being called for the core web plugins? If it's not called somewhere explicitly then the web plugins won't be registered, right?

@imhoffd imhoffd mentioned this pull request Jul 22, 2020
@imhoffd
Copy link
Contributor Author

imhoffd commented Jul 22, 2020

I can add it in, but we're going to be splitting out the plugins anyway in the next few days.

@mlynch
Copy link
Contributor

mlynch commented Jul 22, 2020

Right that makes sense

@imhoffd
Copy link
Contributor Author

imhoffd commented Jul 22, 2020

Okay this is good to go. @jcesarmobile?

@imhoffd imhoffd mentioned this pull request Jul 22, 2020
11 tasks
@imhoffd imhoffd added this to the 3.0.0 milestone Jul 23, 2020
@imhoffd imhoffd merged commit 95475cc into main Jul 23, 2020
@imhoffd imhoffd deleted the plugin-import branch July 23, 2020 18:19
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.

Plugin Import Proposal
3 participants