Make WordPress Core

Opened 8 years ago

Closed 12 months ago

#41498 closed feature request (wontfix)

Why are global functions not wrapped in function_exists?

Reported by: doecode's profile doecode Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: I18N Keywords:
Focuses: Cc:

Description (last modified by SergeyBiryukov)

Some time ago I've written a guide about integrating Laravel into WordPress, https://nehalist.io/integrating-laravel-into-wordpress/. Since Laravel now uses it's own __ function this integration has become impossible, because WordPress does not wrap global functions within function_exists.

Cannot redeclare __() (previously declared in /var/www/test.dev/public_html/wp-laravel-integration/app/vendor/laravel/framework/src/Illuminate/Foundation/helpers.php:821)

Attachments (1)

patch.diff (9.4 KB) - added by jonasemde 6 years ago.
Patch

Download all attachments as: .zip

Change History (13)

#1 @SergeyBiryukov
8 years ago

  • Description modified (diff)

Related: #17268

#2 @SergeyBiryukov
7 years ago

#43645 was marked as a duplicate.

#3 @swissspidy
6 years ago

#45973 was marked as a duplicate.

@jonasemde
6 years ago

Patch

#4 follow-up: @dd32
6 years ago

Is wrapping with function_exists() actually compatible? Wouldn't this cause any sites to loose translations if other-project didn't support the WordPress translations? Shouldn't Laravel wrap it in function_exists() as well? (and loose translation support if imported into WordPress) etc.

#5 @jonasemde
6 years ago

Shouldn't Laravel wrap it in function_exists() as well?

They do this already for all global helper functions included "()": https://github.com/laravel/framework/blob/5.7/src/Illuminate/Foundation/helpers.php

In cases, Laravel is loaded before WordPress inside a composer project an exception is thrown by WordPress.

A function_exists() wrap should solve this issue and not cause any issues on existing sites. Developers are more flexible with this approach.

The same is true for other integrations like:
https://modernmodules.com/plugins/magento-2-wordpress-integration-plugin/

To get this plugin to work you need to hack the /wp-includes/l10n.php file as well. This could be handled a lot cleaner if a function_exists() check is performed.

#6 in reply to: ↑ 4 @jonasemde
6 years ago

#7 follow-up: @swissspidy
6 years ago

Wouldn't this cause any sites to loose translations if other-project didn't support the WordPress translations?

@jonasemde Can you address this question as well?

#8 in reply to: ↑ 7 @jonasemde
6 years ago

This thread is about wrapping the __(...) function to be more flexible as a developer.

Out of the box, this will not cause any issues because if no __(...) function is declared earlier the WordPress default is loaded.

But development wise we gain some flexibility.

This allows initializing a __(...) adapter function before the WordPress PHP-files are loaded. In case of a Laravel/WordPress it could be something like that:

<?php
function __(...$args) {
    // Check if laravel translation exists:
    if(Lang::has(...$args)) {
        return app('translator')->getFromJson(...$args);
    }

    // WordPress as fallback
    return translate(...$args);
}

So this did not cause any sites to lose translations.

Replying to swissspidy:

Wouldn't this cause any sites to loose translations if other-project didn't support the WordPress translations?

@jonasemde Can you address this question as well?

#9 @ocean90
6 years ago

#47584 was marked as a duplicate.

#10 @fishpig
6 years ago

This is something I would recommend also. It provides flexibilty for developers to integrate WordPress into other applications and other applications into WordPress.

Any chance of this happening?

#11 @doncullen
3 years ago

Following up on this, this is still an issue. Any chance of this being patched? It's a pretty easy patch to make, especially for a ticket that has been open for four years.

For now, I've implemented a workaround within Laravel to compensate for Wordpress lack of checking to see if the helper function was already declared. For those wondering, open 'index.php' within the 'public' directory of the Laravel app, and add this before the require __DIR__.'/../vendor/autoload.php'; line:

# Pull in support for WP
require_once '/path/to/your/wp-includes/l10n.php';
require_once '/path/to/your/wp-load.php';
wp_enqueue_style('app', '/yourapp/css/app.css');
wp_enqueue_script('app', '/yourapp/js/app.js');

There's probably better ways to do this, but it's a solution in the meantime.

For those wondering as to what the above solution does, Laravel, unlike WordPress, DOES check to see if the helper function is already declared. So we have Laravel load WordPress's version before loading Laravel's version. So Wordpress's helper is declared first, and Laravel sees it's already declared when it goes to declare it, and doesn't try declaring it -- thereby avoiding a conflict. In other words, the way Wordpress should have done it in the first place.

Last edited 3 years ago by doncullen (previous) (diff)

#12 @swissspidy
12 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to wontfix
  • Status changed from new to closed

WordPress is moving away from pluggable functions like this and we shouldn't be adding new ones. Doing so could lead to all sorts of trouble and backward compatibility issues down the road as we suddenly can't be certain about how __() is used anymore. For example, we would never be able to add additional parameters for things like gender (see #42725). In short, this would put us in a bad spot.

Note: See TracTickets for help on using tickets.