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

Add authorId to internal post URL when it is missing #5825

Merged
merged 1 commit into from
Oct 6, 2024

Conversation

absidue
Copy link
Member

@absidue absidue commented Oct 5, 2024

Add authorId to internal post URL when it is missing

Pull Request Type

  • Feature Implementation

Description

When you paste a post link into the search bar, we need to make two API requests, this first one is to resolve the URL to figure out the channel ID and the second one is to get the post itself (which needs both the post ID and the channel ID).

This pull request adds the authorId to the in-app post URL after the post has loaded, if it was missing from the URL before. That way if you navigate back to the page again in the future e.g. by pressing the back arrow, we only have to make the API request for the post, as we already know the channel ID.

To avoid the replace action triggering the fetching again when we replace the URL, this pull request only watches the id parameter instead of the entire route object.

Screenshots

Before:
before

After:
after

Testing

  1. Paste a post URL into the search bar e.g. https://www.youtube.com/post/UgkxMBvPyJU6K3bhVkR_lI7REEXtt-JoKv36
  2. Wait until the post has loaded
  3. Run window.location.hash in the devtools, you should be able to see both the post ID and the author ID

You can also check the network tab and check that if you navigate back and forth with the arrows in the navigation bar, it should only call both /api/v1/resolveurl and /api/v1/post the first time, after that it should only call /api/v1/post.

Desktop

  • OS: Windows
  • OS Version: 10
  • FreeTube version: 58937d0

@github-actions github-actions bot added the PR: waiting for review For PRs that are complete, tested, and ready for review label Oct 5, 2024
@FreeTubeBot FreeTubeBot enabled auto-merge (squash) October 5, 2024 14:12
@@ -35,7 +35,7 @@ export default defineComponent({
}
},
watch: {
async $route() {
async '$route.params.id'() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm aware this is so the route watch doesnt get triggered when an authorid is added but is there any performance benefit to this as well? Just wondering if we should use this in our other views

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The Vue Router docs do recommend watching only what you need to watch. We can definitely look into doing it in other views too, the hashtag view is an obvious one because that has a single param and no query parameters, but other views like the Search one might be more difficult to narrow down the watcher, as that has a parameter and lots of query parameters.

The route object is a reactive object. In most scenarios, you should avoid watching the whole route object. Instead, you can directly watch the properties you are expecting to change

https://router.vuejs.org/guide/advanced/composition-api.html#Accessing-the-Router-and-current-Route-inside-setup

@FreeTubeBot FreeTubeBot merged commit 056005d into FreeTubeApp:development Oct 6, 2024
5 checks passed
@github-actions github-actions bot removed the PR: waiting for review For PRs that are complete, tested, and ready for review label Oct 6, 2024
@absidue absidue deleted the post-author-id branch October 6, 2024 05:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants