Skip to content

feat: getActionPath()#10353

Merged
sarah11918 merged 13 commits into
5.1.0from
feat/get-action-path
Dec 18, 2024
Merged

feat: getActionPath()#10353
sarah11918 merged 13 commits into
5.1.0from
feat/get-action-path

Conversation

@florian-lefebvre
Copy link
Copy Markdown
Member

Description (required)

Documents a new getActionPath() helper

Related issues & labels (optional)

  • Closes #
  • Suggested label:

@netlify
Copy link
Copy Markdown

netlify Bot commented Dec 12, 2024

Deploy Preview for astro-docs-2 ready!

Name Link
🔨 Latest commit 077d84f
🔍 Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/6762d9063725fc0008561b3e
😎 Deploy Preview https://deploy-preview-10353--astro-docs-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@astrobot-houston
Copy link
Copy Markdown
Contributor

astrobot-houston commented Dec 12, 2024

Lunaria Status Overview

🌕 This pull request will trigger status changes.

Learn more

By default, every PR changing files present in the Lunaria configuration's files property will be considered and trigger status changes accordingly.

You can change this by adding one of the keywords present in the ignoreKeywords property in your Lunaria configuration file in the PR's title (ignoring all files) or by including a tracker directive in the merged commit's description.

Tracked Files

File Note
en/reference/modules/astro-actions.mdx Source changed, localizations will be marked as outdated.
Warnings reference
Icon Description
🔄️ The source for this localization has been updated since the creation of this pull request, make sure all changes in the source have been applied.

Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Thanks @florian-lefebvre ! I left some questions below to help tease out exactly what's happening.

Comment thread src/content/docs/en/reference/modules/astro-actions.mdx Outdated

The `getActionPath()` utility is used to get the path to an action, taking base and trailing slash settings into account. This is useful when you want to call an action for more advanced usages, such as providing custom headers or not using the fetch API (eg. `navigator.sendBeacon`):

```ts
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we have a "this example shows..." and then describe what is happening here?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Is it worth showing 2 examples? Eg. one using the fetch API and one using the navigator.sendBeacon API

```ts
import { actions, getActionPath } from 'astro:actions'

const path = getActionPath(actions.like) // '/_actions/like'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I'm not sure what the commented part is representing. Is that what is returned by getActionPath(actions.like)?

How is the example different using getActionPath(actions.like) vs just actions.like. You mention that often you can just use the action. What is happening here that we're benefiting from using getActionPath() or that wouldn't otherwise be possible?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm not sure what the commented part is representing. Is that what is returned by getActionPath(actions.like)?

Yes!

How is the example different using getActionPath(actions.like) vs just actions.like. You mention that often you can just use the action. What is happening here that we're benefiting from using getActionPath() or that wouldn't otherwise be possible?

See the keepalive option. I guess it's not clear enough tho, so i'll probably use an example with sendBeacon instead

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See the keepalive option. I guess it's not clear enough tho, so i'll probably use an example with sendBeacon instead

Never answer a question with an example! 😄 I need to know, in your words, what is happening so that I can help make them into docs words! You can never assume that someone will get which part of an example is important, or what exactly you're trying to show. Your example might be fine as an illustration! But it's not docs and not an answer to my specific question "How is this example different using getActionPath than not using it?"

Copy link
Copy Markdown
Member Author

@florian-lefebvre florian-lefebvre Dec 16, 2024

Choose a reason for hiding this comment

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

Ha I see (I think)!

How is this example different using getActionPath than not using it?

Calling an action directly does the following under the hood:

await fetch(someInternalPathLogic, {
  method: 'POST',
  headers: {
    'Content-Type': 'application/json',
  },
})

This is internal, which means you can't pass custom headers (eg. you want to protect your actions with a auth header) nor custom options (eg. keepalive which prevents the request from being killed).

Does that help?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

That is very helpful, thank you!

@sarah11918 sarah11918 added add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. merge-on-release Don't merge this before the feature is released! (MQ=approved but WAIT for feature release!) minor-release For the next minor release; in the milestone, "merge queue" = approved for merging on release day. labels Dec 12, 2024
@sarah11918 sarah11918 added this to the 5.1.0 milestone Dec 16, 2024
Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

ugh, so sorry I didn't realize these comments were pending!

```ts
import { actions, getActionPath } from 'astro:actions'

const path = getActionPath(actions.like) // '/_actions/like'
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

See the keepalive option. I guess it's not clear enough tho, so i'll probably use an example with sendBeacon instead

Never answer a question with an example! 😄 I need to know, in your words, what is happening so that I can help make them into docs words! You can never assume that someone will get which part of an example is important, or what exactly you're trying to show. Your example might be fine as an illustration! But it's not docs and not an answer to my specific question "How is this example different using getActionPath than not using it?"

Comment thread src/content/docs/en/reference/modules/astro-actions.mdx Outdated
Comment thread src/content/docs/en/reference/modules/astro-actions.mdx
Copy link
Copy Markdown
Member Author

@florian-lefebvre florian-lefebvre left a comment

Choose a reason for hiding this comment

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

Gotta say I'm struggling to answer correctly your reviews 😄

Comment thread src/content/docs/en/reference/modules/astro-actions.mdx Outdated
```ts
import { actions, getActionPath } from 'astro:actions'

const path = getActionPath(actions.like) // '/_actions/like'
Copy link
Copy Markdown
Member Author

@florian-lefebvre florian-lefebvre Dec 16, 2024

Choose a reason for hiding this comment

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

Ha I see (I think)!

How is this example different using getActionPath than not using it?

Calling an action directly does the following under the hood:

await fetch(someInternalPathLogic, {
  method: 'POST',
  headers: {
    'Content-Type': 'application/json',
  },
})

This is internal, which means you can't pass custom headers (eg. you want to protect your actions with a auth header) nor custom options (eg. keepalive which prevents the request from being killed).

Does that help?

@sarah11918
Copy link
Copy Markdown
Member

Gotta say I'm struggling to answer correctly your reviews 😄

Then they must be the right reviews! 😅

Comment thread src/content/docs/en/reference/modules/astro-actions.mdx Outdated
Comment thread src/content/docs/en/reference/modules/astro-actions.mdx Outdated
Comment thread src/content/docs/en/reference/modules/astro-actions.mdx Outdated
@sarah11918
Copy link
Copy Markdown
Member

So last thing worth mentioning is we no longer mention taking base and trailing slash into account, but I'm wondering whether we have to? That could be "true but not particularly helpful to know" in this context? I mean, you get "the URL path you need for your project" and maybe that's enough? Thoughts?

florian-lefebvre and others added 3 commits December 16, 2024 17:57
Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
@florian-lefebvre
Copy link
Copy Markdown
Member Author

last thing worth mentioning is we no longer mention taking base and trailing slash into account, but I'm wondering whether we have to? That could be "true but not particularly helpful to know" in this context? I mean, you get "the URL path you need for your project" and maybe that's enough? Thoughts?

"The URL path you need for your project" sounds more useful to me, people usually expect trailing slash and base to be taken into account

Comment thread src/content/docs/en/reference/modules/astro-actions.mdx Outdated
Comment thread src/content/docs/en/reference/modules/astro-actions.mdx Outdated
Comment thread src/content/docs/en/reference/modules/astro-actions.mdx
Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

I am really hating GitHub's interface. Here is my stupid review for my stupid pending comment lol. Otherwise, this PR is good to go! 😅

Comment thread src/content/docs/en/reference/modules/astro-actions.mdx Outdated
Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
Comment thread src/content/docs/en/reference/modules/astro-actions.mdx
Comment thread src/content/docs/en/reference/modules/astro-actions.mdx Outdated
florian-lefebvre and others added 2 commits December 18, 2024 15:14
Co-authored-by: Sarah Rainsberger <5098874+sarah11918@users.noreply.github.com>
@sarah11918 sarah11918 changed the base branch from main to 5.1.0 December 18, 2024 14:22
Copy link
Copy Markdown
Member

@sarah11918 sarah11918 left a comment

Choose a reason for hiding this comment

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

Amazing, good to go @florian-lefebvre ! 🥳

@sarah11918 sarah11918 added the Merge Queue Approved and ready to be merged (wait for feature release if also labelled M-O-R)! label Dec 18, 2024
Comment thread src/content/docs/en/reference/modules/astro-actions.mdx Outdated
@sarah11918 sarah11918 merged commit 29fbeb6 into 5.1.0 Dec 18, 2024
@sarah11918 sarah11918 deleted the feat/get-action-path branch December 18, 2024 15:09
@sarah11918 sarah11918 mentioned this pull request Dec 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

add new content Document something that is not in docs. May require testing, confirmation, or affect other pages. Merge Queue Approved and ready to be merged (wait for feature release if also labelled M-O-R)! merge-on-release Don't merge this before the feature is released! (MQ=approved but WAIT for feature release!) minor-release For the next minor release; in the milestone, "merge queue" = approved for merging on release day.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants