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

Android Plugin initialization #1386

Open
GitToTheHub opened this issue Dec 22, 2024 · 6 comments
Open

Android Plugin initialization #1386

GitToTheHub opened this issue Dec 22, 2024 · 6 comments

Comments

@GitToTheHub
Copy link
Contributor

Hi,

when I read the documentation for the Android Plugin initialization the following code is proposed to initialize a plugin:

Plugins should use the initialize method for their start-up logic.

@Override
public void initialize(CordovaInterface cordova, CordovaWebView webView) {
    super.initialize(cordova, webView);
    // your init code here
}

But when I look in the source code of CordovaPlugin#initialize it says that I shouldn't use the initialize method and pluginInitialize instead:

 Called after plugin construction and fields have been initialized.
 Prefer to use pluginInitialize instead since there is no value in
 having parameters on the initialize() function.

Is the documentation outdated?

@erisu
Copy link
Member

erisu commented Dec 24, 2024

The documentation appears to be outdated.

This override example was added 11 years ago (2013), while pluginInitialize was introduced 10 years ago (2014).

Somewhere in between, the initialize method was marked as @Deprecated, but the flag was later removed. I am unsure if @Deprecated was ever part of a production release of the platform.

The commit message states:

Thinking here is that we need a while for both initialize and pluginInitialize to exist before plugin authors would bother not using the deprecated one anyways. Really, no harm in keeping both for some time.

This explains why the @Deprecated flag was removed. However, it seems that this area was never revisited to remove the initialize method, nor was the documentation updated to reflect the introduction of the pluginInitialize method.

Although I was not contributing at the time, I would have suggested retaining the @Deprecated flag. Without the warning, developers might remain unaware of the new method and lack an incentive to make changes. Additionally, the documentation should have been updated to ensure that new plugin developers, especially those building their first plugin, would use pluginInitialize.


I believe we should update the documentation to recommend using pluginInitialize and remove any references to initialize.

Regarding the cordova-android repository, we could consider reintroducing the @Deprecated flag for the upcoming major release. This would give developers a clear warning about the method’s deprecation. Then, in one of the subsequent major releases, we could proceed with removing the initialize method entirely.... Maybe?

Thoughts?

@GitToTheHub
Copy link
Contributor Author

Do you think the use of initialize could be referenced somewhere else in the documentation? I could make a PR to use pluginInitialize instead.

@erisu
Copy link
Member

erisu commented Dec 24, 2024

I tried looking through the documentation to see if there was any other place that might mention the initialize override, but I didn't find any. While there are older versions of the documentation, I don't think it's necessary to update them. If I were to commit this into any version, including dev, I would add it to the current one.

@GitToTheHub
Copy link
Contributor Author

GitToTheHub commented Dec 24, 2024

When you open the documentation, 12.x will always shown to the reader. If you switch manually to dev, it will not remember it. So I would suggest to add it also to 12.x but only, if 13.x will not be published the next time. What do you mean? Or did you mean that :D ?

@erisu
Copy link
Member

erisu commented Dec 24, 2024

That, add it to both 12.x and dev.

Edit: When 13.x is created, it is actually a snapshot of dev. If we add changes only to 12.x, they will not make it into the 13.x documentation.

Also, when it comes to working with documentation, it's usually as simple as making changes to dev and then running npm run update-docs to update the existing snapshot. However, the current dev documentation contains significant changes right now that will only be in 13.x. In this case, it's safer to manually apply the changes to 12.x, but still required in dev.

@GitToTheHub
Copy link
Contributor Author

I created a PR #1387

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

No branches or pull requests

2 participants