Support start timestamps, fix Shorts & Live embed URLs #6234
Conversation
This file contains hidden or 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
🦋 Changeset detectedLatest commit: b337825 The changes in this PR will be included in the next version bump. This PR includes changesets to release 54 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for tiptap-embed ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
@tiptap/core
@tiptap/extension-bold
@tiptap/extension-blockquote
@tiptap/extension-bubble-menu
@tiptap/extension-bullet-list
@tiptap/extension-character-count
@tiptap/extension-code
@tiptap/extension-code-block
@tiptap/extension-code-block-lowlight
@tiptap/extension-collaboration
@tiptap/extension-collaboration-cursor
@tiptap/extension-color
@tiptap/extension-document
@tiptap/extension-floating-menu
@tiptap/extension-focus
@tiptap/extension-dropcursor
@tiptap/extension-font-family
@tiptap/extension-gapcursor
@tiptap/extension-hard-break
@tiptap/extension-heading
@tiptap/extension-highlight
@tiptap/extension-history
@tiptap/extension-horizontal-rule
@tiptap/extension-image
@tiptap/extension-italic
@tiptap/extension-link
@tiptap/extension-list-item
@tiptap/extension-list-keymap
@tiptap/extension-mention
@tiptap/extension-ordered-list
@tiptap/extension-paragraph
@tiptap/extension-placeholder
@tiptap/extension-strike
@tiptap/extension-subscript
@tiptap/extension-superscript
@tiptap/extension-table
@tiptap/extension-table-cell
@tiptap/extension-table-header
@tiptap/extension-table-row
@tiptap/extension-task-item
@tiptap/extension-task-list
@tiptap/extension-text
@tiptap/extension-text-align
@tiptap/extension-text-style
@tiptap/extension-typography
@tiptap/extension-underline
@tiptap/extension-youtube
@tiptap/html
@tiptap/react
@tiptap/pm
@tiptap/starter-kit
@tiptap/suggestion
@tiptap/vue-2
@tiptap/vue-3
commit: |
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the YouTube embed URL generation, adding support for start timestamps and fixing embed issues for Shorts and Live URLs.
- Refactors URL processing by using the native URL object
- Converts various timestamp formats to the standardized "start" parameter
- Fixes embed issues for Shorts and Live YouTube URLs
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/extension-youtube/src/utils.ts | Refactors embed URL construction and parameter mapping for YouTube URLs |
| .changeset/clever-hats-count.md | Updates changeset metadata to reflect the PR changes |
|
|
||
| if (params.length) { | ||
| outputUrl += `${matches[1] === 'v' ? '?' : '&'}${params.join('&')}` | ||
| embedUrl.searchParams.set('rel', rel.toString()) |
There was a problem hiding this comment.
Setting 'rel' without first checking if it is defined could cause a runtime error when 'rel' is undefined. Consider adding a condition to set this parameter only when defined.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
Line 166 literally checks if rel is not undefined.
| } | ||
|
|
||
| if (startAt) { | ||
| params.push(`start=${startAt}`) | ||
| embedUrl.searchParams.set('start', startAt.toString()) |
There was a problem hiding this comment.
[nitpick] Both the URL's 't' parameter and the 'startAt' option set the 'start' query parameter, which may lead to unintended behavior if both are provided. Consider clarifying which parameter should take precedence or add a check to avoid conflict.
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
This startAt here is custom set by the developer. This must take precedence over t or start params retrieved from the URL. And searchParams.set will set a given param if not already. This code already represents the intended behaviour.
|
Didn't expect I'll get a review from AI, but doesn't seem like it understands the subtleties of a(n already) correct code 😆 @arnaugomez Back to you. Let me know if there are changes requested! |
yangshun
reviewed
Apr 20, 2025
| if (!matches || !matches[2]) { | ||
| return null | ||
| if (urlObject.searchParams.has('t')) { | ||
| embedUrl.searchParams.set('start', urlObject.searchParams.get('t')!.replaceAll('s', '')) |
There was a problem hiding this comment.
Refactoring of the URL building should ideally be done in a separate PR.
| @@ -34,6 +34,28 @@ export const getYoutubeEmbedUrl = (nocookie?: boolean, isPlaylist?:boolean) => { | |||
| return nocookie ? 'https://www.youtube-nocookie.com/embed/' : 'https://www.youtube.com/embed/' | |||
| } | |||
|
|
|||
| const getYoutubeVideoOrPlaylistId = ( | |||
There was a problem hiding this comment.
Would be good to add some tests as a lot of code has been changed.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Start timestamps are currently not embedded with
youtu.beandyoutube.com/watchURLs. There are also inconsistencies with howyoutu.beandyoutube.com/watchURLs are parsed, and with how search params in the original URL are being handled.This PR refactors the
getEmbedUrlFromYoutubeUrlfunction to use the native URL object to create the embed URL. It also converts the start timestamps liket=10inyoutu.beandt=10sinyoutube.com/watchtostart=10inyoutube.com/embedURLs. This should resolve #5765.As an added bonus, it also appears that Shorts URLs are not embedded correctly. It's fixed in this PR. While I'm at it, I also added support for Live URLs, so this PR closes #5447 too.
Checklist