withastro / docs

Astro documentation
https://docs.astro.build/
MIT License
1.23k stars 1.33k forks source link

Explain `Astro.glob()` doesn't accept variables #550

Closed tinymachine closed 1 year ago

tinymachine commented 2 years ago

I'm seeing a lot of folks in the Discord get tripped up on this (example), so I thought it would warrant a note in the documentation. Please feel free to edit as necessary.

What kind of changes does this PR include?

Description

netlify[bot] commented 2 years ago

Deploy Preview for astro-docs-2 ready!

Built without sensitive environment variables

Name Link
Latest commit 3e6b6c18dbb1297b605cc4c218381b80f14d11a7
Latest deploy log https://app.netlify.com/sites/astro-docs-2/deploys/62d8220517c6330009830ab0
Deploy Preview https://deploy-preview-550--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 settings.

sarah11918 commented 2 years ago

Thank you for this @tinymachine ! This looks like a really helpful example, and we'll get on incorporating this into the docs very soon!

tony-sull commented 2 years ago

Not sure what the build error in Netlify is after updating to main 🤔

delucis commented 2 years ago

Not sure what the build error in Netlify is after updating to main 🤔

Looks like it’s trying to use pnpm and failing to find it? I noticed the builds were running with yarn for some reason so this looks like a change. Has someone changed that over in the Netlify config?

Maybe we need to look at the set up we currently recommend for Netlify + pnpm in the docs (but don’t actually use 😅): https://docs.astro.build/en/guides/deploy/#netlifytoml-file

tony-sull commented 2 years ago

Ah yeah the last run was me changing it to use PNPM to see if that was the issue, this was the actual build error that I'm trying to track down

tony-sull commented 2 years ago

Sorry for the extra noise while I was debugging a build error in our PR builds!

We actually have a separate PR open right now to add a troubleshooting guide that should be ready to merge in soon. This example would be perfect for that guide, would you mind holding on this PR until the troubleshooting guide is merged so the example can live on that page instead of the API reference?

sarah11918 commented 2 years ago

@tinymachine Can you please change this to a "Draft" PR, since we do want to wait for #536 to merge first. (And, THAT ONE is still a draft too, so it's going to be a few days at least!)

Then, when we have that new Troubleshooting page, I'd like to revisit this edit, but put it on THAT page instead of the one suggested here. So, this is on my to-do list, but it will probably be next week that you see movement on this.

Marking it as Draft in the mean time helps me keep organized, and stops me from constantly reopening to check in on what I need to do with THIS PR! Thank you!

sarah11918 commented 2 years ago

UPDATE: Converted this to a draft myself, but don't worry... we WILL get back to this!

sarah11918 commented 1 year ago

@tinymachine Just checking in on this! Are you still interested in adding it to the Troubleshooting page?

tinymachine commented 1 year ago

Sorry for delay, @sarah11918! If the functionality of Astro.glob() hasn't changed since I submitted the PR, I think this could still be helpful for folks, so I think it's worth adding. Let me know if you think otherwise!

delucis commented 1 year ago

Totally! No changes to the API, so this would still be useful to add to our new Troubleshooting page! Thanks @tinymachine.

sarah11918 commented 1 year ago

I've just removed the draft label, as the other PR was merged so we should be able to tackle the content now.

In the meantime, I know some content related to this was added here: https://docs.astro.build/en/guides/troubleshooting/#supported-values

I'm sorry, I think a different request for similar content arose, and with this still having been marked as a draft, it escaped my mind that we had similar content already here waiting! So, please take a look at what's currently on our troubleshooting page there and let us know if either that works well enough for you, or whether you'd like to PR an edit to that. Thanks for your patience!

tinymachine commented 1 year ago

So sorry, @delucis and @sarah11918 -- I drafted a response to @sarah11918's question earlier and neglected to post it! I think your edit makes sense, @delucis, and linking to the existing content on the troubleshooting guide is great.

FWIW I think the code example in the troubleshooting guide isn't quite as simple as it could be -- having part of the pathToMyFeaturedPost in the example include the postSlug taken from the props kind of complicates the issue in question here, in my opinion. I ended up having to reason a bit about what the use case was in that code sample, which I think is somewhat beside the point. But if it's a common use case, then it makes sense to keep it as-is.

Thanks again for keeping me in the loop -- I really appreciate it!

delucis commented 1 year ago

Thanks again @tinymachine! I think your comment on that code example is fair, so I’d be happy to accept a PR if you’d like to propose some simplifications 🙌