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

CLI paths take the PC Root, not the project's path root. #8831

Closed
1 task
muchisx opened this issue Oct 13, 2023 · 8 comments
Closed
1 task

CLI paths take the PC Root, not the project's path root. #8831

muchisx opened this issue Oct 13, 2023 · 8 comments
Labels
- P2: nice to have Not breaking anything but nice to have (priority) feat: dev Related to `astro dev` CLI command (scope) help wanted Please help with this issue!

Comments

@muchisx
Copy link

muchisx commented Oct 13, 2023

Astro Info

"Astro                    v3.2.2\nNode                     v18.16.0\nSystem                   Windows (x64)\nPackage Manager          npm\nOutput                   server\nAdapter                  @astrojs/vercel/serverless\nIntegrations             @custom/update-webflow-js\n                         @astrojs/tailwind\n                         @astrojs/preact\n                         @astrojs/prefetch\n                         astro-robots-txt"

If this issue only occurs in one browser, which browser is a problem?

No response

Describe the Bug

The Astro CLI during npm run dev is using my entire pc path for outputs, can this be modified to instead use the project's root path?

image

What's the expected result?

I would think the project's root path is better! instead of the entire PC path !

Link to Minimal Reproducible Example

https://stackblitz.com/edit/github-n2mknz?file=README.md

Participation

  • I am willing to submit a pull request for this issue.
@github-actions github-actions bot added the needs triage Issue needs to be triaged label Oct 13, 2023
@lilnasy
Copy link
Contributor

lilnasy commented Oct 16, 2023

Seems to be the full path only on windows.

image

@lilnasy lilnasy added help wanted Please help with this issue! - P2: nice to have Not breaking anything but nice to have (priority) feat: dev Related to `astro dev` CLI command (scope) and removed needs triage Issue needs to be triaged labels Oct 16, 2023
@florian-lefebvre
Copy link
Member

@lilnasy is this logic happening here

const file = ctx.file.replace(config.root.pathname, '/');
? Looks like it's replaced already so not sure what's going on

@lilnasy
Copy link
Contributor

lilnasy commented Oct 16, 2023

Thanks for looking into it!

I think either ctx.file or config.root.pathname needs URI decoding ("%20" into the space character.)

If that is the case, I would expect the full path to appear when there are special characters in the path on any OS.

@florian-lefebvre
Copy link
Member

I have also the same thing right now with no space nor special character in the path, not sure to understand what you mean 🤔

@lilnasy
Copy link
Contributor

lilnasy commented Oct 16, 2023

Nevermind, it's not that. When pathname is read from a URL object (config.root in this case), it starts with a forwards slash, which prevents replace's pattern from matching.

image

@lilnasy
Copy link
Contributor

lilnasy commented Oct 16, 2023

...in addition to the undecoded URI, which affects non-Windows platforms as well.
image

@florian-lefebvre
Copy link
Member

Not sure where to look then 🤔, I'm not familiar enough with the codebase

@lilnasy
Copy link
Contributor

lilnasy commented Oct 17, 2023

@florian-lefebvre You already got it actually. The hmr.ts line you linked is the appropriate place. You might find fileUrlToPath function from "node:url" helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P2: nice to have Not breaking anything but nice to have (priority) feat: dev Related to `astro dev` CLI command (scope) help wanted Please help with this issue!
Projects
None yet
Development

No branches or pull requests

3 participants