-
Notifications
You must be signed in to change notification settings - Fork 107
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
Allow extensions to opt-in to using the web-vitals attribution build via the od_use_web_vitals_attribution_build
filter
#1759
Merged
Merged
Changes from 38 commits
Commits
Show all changes
45 commits
Select commit
Hold shift + click to select a range
fc95f4a
Bundle web-vitals attribution build
swissspidy 651577e
Allow loading web-vitals attribution build
swissspidy ff3f120
Collect some INP data for testing
swissspidy 9dd8159
Add debugging helper for LCP elements
swissspidy 12a3494
some lint fixes
swissspidy be0be49
Add INP elements with JS
swissspidy e9bbe99
Apply suggestions from code review
swissspidy ac4d153
Add missing return statement
swissspidy 098191c
Lint fixes
swissspidy 9288025
Clarify comment
swissspidy 2f507a0
PHPStan fixes
swissspidy 8ead9ae
Multiline comment
swissspidy ad09dea
Merge branch 'trunk' into try/1736-debug-detective
swissspidy 3e4d648
Add empty line
swissspidy c8b490b
Merge branch 'trunk' into try/1736-debug-detective
swissspidy 36430ad
Only load assets if admin bar is showing
swissspidy a229ba6
Extract `od_get_group_collection()` helper
swissspidy ba48bd9
Don't add visitor if not showing admin bar
swissspidy 581ffbc
Add `additionalProperties`
swissspidy eabd8c8
Fix anchors and popovers
swissspidy ebfea3f
Re-enable css
swissspidy c2d1a9f
Fix test_get_json_schema
westonruter 6d95ce1
Prevent warning if INP data is missing
swissspidy 5de84c6
Remove admin bar condition in admin bar cb
swissspidy adf3ae5
Styling adjustments
swissspidy e7432ed
Remove dev mode condition
swissspidy d552366
Move INP logic to tag visitor too
swissspidy 2aaad52
Undo `od_get_group_collection`
swissspidy 39f8336
Lint fixes
swissspidy 1d900d2
Reuse existing anchor name
swissspidy efc1aa6
phpstan fixes
swissspidy b63b9d2
lint fix
swissspidy 6d848d5
Merge branch 'trunk' into try/1736-debug-detective
swissspidy 2f162f2
Remove extracted code
swissspidy 89b718a
Merge branch 'trunk' into try/1736-debug-detective
swissspidy d32ffc0
Undo tests change too
swissspidy 2802b57
Remove JS parts now as well
swissspidy 5fd6891
Undo type import
swissspidy 8dd8af6
Don't conditionally load by default
swissspidy c6f53fa
Remove newline
swissspidy fd7fa9f
Add jsdoc description
westonruter 3614c13
Add typing for attribution report functions
westonruter 3a30027
Add docs for the od_use_web_vitals_attribution_build filter
westonruter a75dd53
Add test for od_use_web_vitals_attribution_build filter
westonruter 9600078
fixup! Add docs for the od_use_web_vitals_attribution_build filter
westonruter File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could this go back to being defined inline? The type is picked up for me in PhpStorm:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But your PhpStorm clearly only sees
LCPMetric
, notLCPMetricWithAttribution
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point!
What's strange to me is why this typing is even needed.
With 3614c13 I added more typing for the attribution report functions. Then PHPStorm reports that the arg is detected as a
LCPMetricWithAttribution
:But TypeScript is giving a warning about it being implicit
any
:Nevertheless,
onLCP
is defined as being eitherOnLCPFunction
orOnLCPWithAttributionFunction
:So I would have thought that this would be later be reflected in the type of the arg here. But instead it is showing up as
any
, even though the commonReportOpts
arg is carried through: