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

Shut down the classes surface #19322

Open
5 tasks
jdevalk opened this issue Dec 1, 2022 · 4 comments
Open
5 tasks

Shut down the classes surface #19322

jdevalk opened this issue Dec 1, 2022 · 4 comments

Comments

@jdevalk
Copy link
Contributor

jdevalk commented Dec 1, 2022

The classes part of the YoastSEO() surface basically makes our entire code base public, something we don't think is maintainable, so we have to shut it down.

I've been doing some analysis as to how people use that surface right now.

Current usage

1. Getting the repository

I've seen the Web Stories plugin, the Visual Composer plugin and aHrefs SEO all do this:

$repository = YoastSEO()->classes->get( 'Yoast\WP\SEO\Repositories\Indexable_Repository' );

2. Getting the Indexable builder

The Web Stories plugin on top of that uses the builder, we should probably find out why ( @swissspidy, can you enlighten us? :) )

$builder = YoastSEO()->classes->get( 'Yoast\WP\SEO\Builders\Indexable_Builder' );

3. Shutting down Yoast SEO output

The Events Made Easy plugin and Expertfile plugin both do this:

$front_end = YoastSEO()->classes->get( Yoast\WP\SEO\Integrations\Front_End_Integration::class );
remove_action( 'wpseo_head', [ $front_end, 'present_head' ], -9999 );

4. Internal uses in Yoast SEO

We still use this surface in multiple places inside Yoast SEO because we don't do DI properly everywhere yet. We should fix that before deprecating.

Proposed solution

We should:

  • create an API to get the repository (fixes 1. above)
  • find a fix for 2. above in communication with @swissspidy
  • create a filter to disable Yoast SEO output (fixes 3. above)
  • Replace all the uses of YoastSEO()->classes inside the codebase with a new function marked private explicitly, fixes 4.
  • deprecate the use of the classes surface and remove all classes not listed above from what it returns so we don't create new problems
@swissspidy
Copy link
Contributor

👋

~2yrs ago we changed our plugin's rewrite slug from /stories/ to /web-stories/.

That caused issues for some of our users who were also using Yoast SEO, because Yoast SEO caches the permalinks in the database. That meant stories created prior to that change had outdated canonical URLs and the like.

To solve that, we added a custom upgrade routine to our plugin for a one-time index rebuild of all stories with outdated rewrite slugs. For newer sites it's effectively a no-op.

At the time, using the builder like this was the recommended approach.

We also added some safeguards to see if the classes and functions we're using from Yoast SEO really exist.

So if you would simply remove those from your code base, I think we should be safe.

@jdevalk
Copy link
Contributor Author

jdevalk commented Dec 1, 2022

thanks @swissspidy !! But... if we deprecate the endpoint, you might get deprecation notices... Maybe you could consider just removing the code in a future version? :)

@swissspidy
Copy link
Contributor

Sure, yeah, we will look into that

@swissspidy
Copy link
Contributor

The code will be removed in our next release

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