-
Notifications
You must be signed in to change notification settings - Fork 27.4k
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
Fix sendGAEvent function params and type clearly #63226
Open
hjick
wants to merge
9
commits into
vercel:canary
Choose a base branch
from
hjick:canary
base: canary
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+237
−5
Open
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
c99a807
Update GA send event function params and add docs example
hjick 7319e58
Update docs gtag link to GA4
hjick df93029
Update next GAEvent docs example
hjick e5ddfd0
Merge branch 'vercel:canary' into canary
hjick c077f70
refactor: add more strict types on sendGAEvent function parameters
hjick 5d388eb
Merge branch 'canary' into canary
hjick db8e87f
chore: run pnpm prettier-fix
hjick 48d0315
Merge branch 'canary' into canary
hjick 7e56f0b
Merge branch 'canary' into canary
hjick 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
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.
I think this type is too restrictive.
Some gtag events take an
items
parameter, which is an array of Item objects.https://developers.google.com/tag-platform/gtagjs/reference/events
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.
Thank you for your review!
The url you attached return 500 status. Is that url about gtag ```event``` command?I can't find that
items
parameters in recent docs about gtag.https://developers.google.com/tag-platform/gtagjs/reference#event
If the
items
parameter you're talking about is a gtag command, how about creating a function for each command separately?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.
I added type GA events include
items
parameters you referenced.If the event name entered by the user is the same as the event name in the link you referenced, it also provides autocomplete event parameters.
Can you check this changes?
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.
I think your types won't allow custom events. Maybe there is an existing official set of types from Google that we could use?
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.
The type allow custom events. And I also added types from Google docs.
And if user input types from Google official types, user can get autocomplete Google official params.
Also user can input custom types.
I pushed new commit. Please check that.
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.
Thank you for your maintaining. I know you're busy, but could you please check one more time, this commit would be very helpful.