LRC script implementation

[wordpressfan]

Description

This PR has the changes that we do here: wp-media/rocket-scripts#20

Documentation

User documentation

Identifies correctly elements to apply LRC optimization

Technical documentation

We are getting the hashes of the eligible candidates to apply LRC opt on and returning the payload together with LCP candidates to PHP for processing.

Type of change

Delete options that are not relevant.

• New feature (non-breaking change which adds functionality).
• Enhancement (non-breaking change which improves an existing functionality).

New dependencies

N/A

Risks

N/A

Checklists

Feature validation

• I validated all the Acceptance Criteria. If possible, provide sreenshots or videos.
• I triggered all changed lines of code at least once without new errors/warnings/notices.
• I implemented built-in tests to cover the new/changed code.

Documentation

• I prepared the user documentation for the feature/enhancement and shared it in the PR or the GitHub issue.
• The user documentation covers new/changed entry points (endpoints, WP hooks, configuration files, ...).
• I prepared the technical documentation if needed, and shared it in the PR or the GitHub issue.

Code style

• I wrote self-explanatory code about what it does.
• I wrote comments to explain why it does it.
• I named variables and functions explicitely.
• I protected entry points against unexpected inputs.
• I did not introduce unecessary complexity.

[codacy-production]

Coverage summary from Codacy

See diff coverage on Codacy

Coverage variation	Diff coverage
Report missing for a560c6b1	✅ ∅ (target: 50.00%)

Coverage variation details

	Coverable lines	Covered lines	Coverage
Common ancestor commit (a560c6b)	Report Missing	Report Missing	Report Missing
Head commit (31c145e)	37849	14455	38.19%

Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: <coverage of head commit> - <coverage of common ancestor commit>

Diff coverage details

	Coverable lines	Covered lines	Diff coverage
Pull request (#6874)	0	0	∅ (not applicable)

Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: <covered lines added or modified>/<coverable lines added or modified> * 100%

See your quality gate settings    Change summary preferences

_{Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more}

Footnotes

1. Codacy didn't receive coverage data for the commit, or there was an error processing the received data. Check your integration for errors and validate that your coverage setup is correct. ↩

[jeawhanlee]

Payload received by PHP:

stdClass Object
(
    [lcp] => Array
        (
            [0] => stdClass Object
                (
                    [type] => bg-img
                    [src] => https://fastly.picsum.photos/id/10/2500/1667.jpg?hmac=J04WWC_ebchx3WwzbM-Z4_KC_LeLBWr5LZMaAkWkF68
                    [srcset] => 
                    [sizes] => 
                    [sources] => Array
                        (
                        )

                    [bg_set] => Array
                        (
                            [0] => stdClass Object
                                (
                                    [src] => https://fastly.picsum.photos/id/10/2500/1667.jpg?hmac=J04WWC_ebchx3WwzbM-Z4_KC_LeLBWr5LZMaAkWkF68
                                )

                        )

                    [current_src] => 
                    [label] => lcp
                )

            [1] => stdClass Object
                (
                    [type] => bg-img
                    [src] => https://fastly.picsum.photos/id/12/2500/1667.jpg?hmac=Pe3284luVre9ZqNzv1jMFpLihFI6lwq7TPgMSsNXw2w
                    [srcset] => 
                    [sizes] => 
                    [sources] => Array
                        (
                        )

                    [bg_set] => Array
                        (
                            [0] => stdClass Object
                                (
                                    [src] => https://fastly.picsum.photos/id/12/2500/1667.jpg?hmac=Pe3284luVre9ZqNzv1jMFpLihFI6lwq7TPgMSsNXw2w
                                )

                        )

                    [current_src] => 
                    [label] => above-the-fold
                )

            [2] => stdClass Object
                (
                    [type] => img
                    [src] => http://localhost:10003/rolling-hills.jpg
                    [srcset] => 
                    [sizes] => 
                    [sources] => Array
                        (
                        )

                    [bg_set] => Array
                        (
                        )

                    [current_src] => http://localhost:10003/rolling-hills.jpg
                    [label] => above-the-fold
                )

        )

    [lrc] => Array
        (
            [0] => 3ead51141d9876165378a22a92d90415
            [1] => eb156891061284662641900bc9136fae
            [2] => 1b496e8f8129c0eb6eda409a2b17c24d
        )

)

[jeawhanlee]

I have tested that the beacon script works as expected with the AJAX PR from here that is now merged

@MathieuLamiot However one thing though, QA will not be able to test this without having errors as there is dependency here on the frontend, but I have a temporary workaround for this but this will make tests fail here on this PR, do you want me to do that and unlock this part for @wp-media/qa-team to test?

[MathieuLamiot]

I can't say without more details about this error/dependency. IDeally, there should be no errors nor dependency, and tests should be passing. If this is not the case and this is not achievable, we can assess ; but we'll need more details

[jeawhanlee]

The concrete factory which should be added in the performancehints ServiceProvider isn't there yet because the frontend controller is not completely ready and passed to that same factory constructor here that's what I mean.

[MathieuLamiot]

So, what is the impact exactly? Is the error thrown but all features implemented and merged so far are OK?

• hashes are added to the page
• beacon is injected when needed (ie. not injected if both features are off, or if data is already available)
• etc.
  If it is just an error that we can discard, but everything is working as expected, I would say this is fine.

[jeawhanlee]

There's no error thrown but only the ATF feature works completely while the LRC feature will run partially

• Hashes will be generated
• LRC Beacon will not run because of the factory issue I stated above rather it will output this to the console:
  Not running BeaconLrc because data is already available or feature is disabled because again the frontend responsible for setting config.status.lrc is not ready yet.
• LRC Table will be created.
• Ajax will not run because the LRC factory is not registered in WP_Rocket\Engine\Common\PerformanceHints\ServiceProvider yet

This is why I said, I have a workaround for this missing factory if the beacon and Ajax needs to be tested by QA, I can put the steps to test these the way I did, else, we wait for the frontend to be completed.

[MathieuLamiot]

Ok thanks, so the goal is to make sure we track everything that is still needed. From what I see, apart from some bugs reported by QA, the only GH issue we still have are:

• Applying the front-end optimization (on-going) ;
• Managing deactivation by filters and impact of the clear data buttons.

So the two defects you noticed (LRC Beacon not running & AJAX not running) are not covered by those GH issues. Therefore we need:
1 - To create GH issues stating the issue we currently have on the feature branch -> @jeawhanlee can you do it?
2 - Add them to the board, current sprint.
3 - Implement a PR to fix this.
I'll let you assess if we need 1 or 2 separate issues.

[MathieuLamiot]

Conclusion of discussion with @jeawhanlee: There are 2 defects preventing to happily run the LRC Beacon and related AJAX calls (so impacting #6874 & #6828). @jeawhanlee mentioned it should be a quick fix, he'll open a PR to fix this and ultimately allow testing of those 2 GH issues.
(cc @wp-media/qa-team)

[MathieuLamiot]

Approved as it is the WP Rocket counter part of the already approved Rocket-scripts PR

[Mai-Saad]

If this PR is about optimize the correct elements, we aren't considering now the child who is in threshold but parent not in threshold wp-media/rocket-scripts#25 (comment)