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

New backend section to handle the Automatic Lazy Rendering exclusions #6987

Closed
piotrbak opened this issue Sep 20, 2024 · 6 comments · Fixed by #6988
Closed

New backend section to handle the Automatic Lazy Rendering exclusions #6987

piotrbak opened this issue Sep 20, 2024 · 6 comments · Fixed by #6988
Assignees
Labels
effort: [M] 3-5 days of estimated development time module: ALR Issues related to the Automatic Lazy Rendering feature priority: high Issues which should be resolved as quickly as possible type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Milestone

Comments

@piotrbak
Copy link
Contributor

piotrbak commented Sep 20, 2024

Is your feature request related to a problem? Please describe.
We have some different issues related to the layout problems with Automatic Lazy Rendering:
#6959

Describe the solution you'd like
We'd like to be able to define HTML elements which will not receive hashes during the process (exclude from the feature). It'd allow us to exclude elements before the real fix is applied.

It should work with regex.

Another approach
Prevent from adding data-wpr-lazyrender attribute to the elements

Additional context
We should have a new backend section to handle it

@piotrbak piotrbak added type: enhancement Improvements that slightly enhance existing functionality and are fast to implement priority: high Issues which should be resolved as quickly as possible module: ALR Issues related to the Automatic Lazy Rendering feature labels Sep 20, 2024
@MathieuLamiot
Copy link
Contributor

Discussed with @piotrbak: this would be released as a hotfix, so not including other changes already on develop. To take into account for branch management.

@Miraeld
Copy link
Contributor

Miraeld commented Sep 20, 2024

We should have a new backend section to handle it

Where is it supposed to be in the backend ?

@MathieuLamiot
Copy link
Contributor

From what I understand, there will be a new dedicated section: https://github.com/wp-media/rucss-backend/issues/140

@piotrbak piotrbak added this to the 3.17.0.2 milestone Sep 20, 2024
@wordpressfan
Copy link
Contributor

Grooming

Add new method here:

/**
 * Get the lazy rendered exclusions.
 *
 * @return array
 */
public function get_lrc_exclusions(): array {
	$lists = $this->providers['defaultlists']->data_manager->get_lists();

	return $lists->lazy_rendering_exclusions ?? [];
}

Then we need to get those exclusions and use them inside each processor class (Currently we will start with the Dom processor class only)

In Dom class, exactly here:

Add the following lines:

private $exclusions = [];

public function set_exclusions( $exclusions = [] ) {
	$this->exclusions = $exclusions;
}

private function get_exclusions() {
	return $this->exclusions;
}

and exactly here:

$opening_tag_html = strstr( $child_html, '>', true ) . '>';

Loop on the exclusions list if found and use the pattern like the one here:

private function get_inline_exclusions_list_pattern() {
$inline_exclusions_list = $this->get_inline_exclusions();
/**
* Filters the patterns used to find inline JS that should not be deferred
*
* @since 3.8
*
* @param array $inline_exclusions_list Array of inline JS that should not be deferred.
*/
$additional_inline_exclusions_list = apply_filters( 'rocket_defer_inline_exclusions', [] );
$inline_exclusions = '';
// Check if filter return is string so convert it to array for backward compatibility.
if ( is_string( $additional_inline_exclusions_list ) ) {
$additional_inline_exclusions_list = explode( '|', $additional_inline_exclusions_list );
}
// Cast filter return to array.
$inline_exclusions_list = array_merge( $inline_exclusions_list, (array) $additional_inline_exclusions_list );
foreach ( $inline_exclusions_list as $inline_exclusions_item ) {
$inline_exclusions .= preg_quote( (string) $inline_exclusions_item, '#' ) . '|';
}
return rtrim( $inline_exclusions, '|' );
}

to be used in preg_match and bailout if no match.

When setting processor, exactly here:

public function set_processor( $processor ): void {

We can apply a new filter like:

$exclusions = wpm_apply_filters_typed( 'string[]', 'rocket_lrc_exclusions', [] );

Then below the Dom instantiation line, we can call:

$this->processor->set_exclusions( $exclusions );

Then we need to add a callback for that new filter to get the exclusions from the dynamic list, we can do that here:

'rocket_staging_list' => 'add_staging_exclusions',

'rocket_lrc_exclusions' => 'add_lrc_exclusions'

Then create this new method:

public function add_lrc_exclusions( $exclusions ) {
	return $this->dynamic_lists->get_lrc_exclusions();
}

Effort:

S to M including tests time.

@Miraeld
Copy link
Contributor

Miraeld commented Sep 23, 2024

Looks good to me

@MathieuLamiot
Copy link
Contributor

Then, do not hesitate to move it to "Todo" @Miraeld ;)

@MathieuLamiot MathieuLamiot added the effort: [M] 3-5 days of estimated development time label Sep 23, 2024
@remyperona remyperona self-assigned this Sep 23, 2024
@hanna-meda hanna-meda self-assigned this Sep 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
effort: [M] 3-5 days of estimated development time module: ALR Issues related to the Automatic Lazy Rendering feature priority: high Issues which should be resolved as quickly as possible type: enhancement Improvements that slightly enhance existing functionality and are fast to implement
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants