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

Astro CLI --root with build not behaving as expected -- seemingly borks import paths #7934

Closed
1 task
firxworx opened this issue Aug 3, 2023 · 7 comments · Fixed by #7977
Closed
1 task
Assignees
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: monorepo Related to usage within a monorepo (scope)

Comments

@firxworx
Copy link

firxworx commented Aug 3, 2023

What version of astro are you using?

^2.6.5

Are you using an SSR adapter? If so, which one?

No

What package manager are you using?

pnpm

What operating system are you using?

Ubuntu on WSL2

What browser are you using?

Chrome

Describe the Bug

Many thanks to the community for Astro! I'm not sure if this is an actual CLI issue or simply a documentation issue for --root:

https://docs.astro.build/en/reference/cli-reference/#--root-path

I am not sure because until now I have only used Astro via the @nxtensions/astro plugin for Nx monorepos. From my initial investigation it feels like the issue might be upstream with the Astro CLI so I am reporting here.

Here is the issue report at @nxtensions/astro where I provide details including replication of a build issue caused by specifying the --root: nxtensions/nxtensions#445

A copy and paste of the key Astro CLI commands from that issue to this one for convenience:

# this will seemingly build but take careful note of the output vs. the error output below
pnpm astro build --config apps/op/astro-ui/astro.config.mjs

# this exactly replicates the error seen when using `pnpm nx build ...` !!!
pnpm astro build --root apps/op/astro-ui --config astro.config.mjs

# after ensuring outDir, srcDir, and publicDir are all explicitly specified the error goes away with the following
pnpm astro build --config apps/op/astro-ui/astro.config.mjs

It seems that specifying --root is internally borking the relative paths to imports during the build. By avoiding this argument and only specifying a path to --config that explicitly defines each required path seems to work around / fix the problem.

Again, not sure if this is really a documentation issue that isn't explicit/clear about how --root behaves and what it actually does or if its a bug with the astro build command itself, however I'm pretty sure there is an issue.

What's the expected result?

I would not have expected --root to have the impact on path resolution that it apparently has with the latest version of the CLI.

Sorry for "cheating" and putting a link to the docs in the "Minimal Reproducible Example"; per my report the docs may genuinely be the issue. As far as I can tell my experience can be reproduced in any current astro CLI with the --root flag used as described.

Link to Minimal Reproducible Example

https://docs.astro.build/en/reference/cli-reference/#--root-path

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 Aug 3, 2023
@natemoo-re
Copy link
Member

Curious if this might have been caused by #7647 or if this has been an issue for a while and we just never caught it before!

cc @bluwy

@natemoo-re natemoo-re added feat: monorepo Related to usage within a monorepo (scope) - P4: important Violate documented behavior or significantly impacts performance (priority) labels Aug 3, 2023
@github-actions github-actions bot removed the needs triage Issue needs to be triaged label Aug 3, 2023
@firxworx
Copy link
Author

firxworx commented Aug 3, 2023

Thanks @natemoo-re I suspect you are onto something.

Update:

I may have spoken too soon about the viability of the workaround/fix.

As I added some tailwind classes not seen elsewhere in my project (to flex postcss which clears unused tailwind classes from the build) and I was able to encounter/trigger various more issues.

Its probably not of value to share the details because the ultimate conclusion is:

There's definitely some wonky path fun happening with the current Astro CLI version where there was not before.
Even when the errors I can trigger have completely obtuse error messages I strongly suspect they are 100% related to borked paths.

As part of my additional experimentation, I added a postcss config to the root of my project monorepo (where there shouldn't be one: each app with tailwind has its own config in its respective app folder), and added to it a console warning that it was getting triggered vs. the app's version. Sure enough I can get it to fire warnings in various cases.

I can get odd behaviours in pretty much every case I try both with and without --root.

If I set --root to my app folder and even if I pass a configPath to tailwind() Astro plugin and even if I replace __dirname and the likes with real paths built from path.resolve() (and using Nx's devtools to obtain workspaceRoot). Pretty much no matter what I do I can find some evidence somewhere of something not working right with high suspicion that the root cause is a borked path.

I'm certain that anyone will be able to replicate issues and my fingers are crossed that the devs working on these recent new features might recognize exactly what is going on :)

I haven't tried it to confirm but I bet a stock repo with just Astro i.e. the "happy path" works and anything that deviates from it is where the issues are.


In case it helps: I have a project with older Astro and older related version of the nx plugin that I'm using to support it within the Nx workspace and everything works great with the identical configuration over there.

Relevant versions for working project are:

    "@astrojs/markdown-remark": "^2.2.1",
    "@astrojs/react": "^2.2.1",
    "@astrojs/sitemap": "^1.3.2",
    "@astrojs/tailwind": "^3.1.3",
    "@nxtensions/astro": "3.5.0",
    "astro": "^2.5.6",

The affected repo with this issue is "^2.6.5".
Take note quick skim readers: same digits but very different minor + patch versions :)

@firxworx
Copy link
Author

firxworx commented Aug 3, 2023

Confirming same issues with the latest release astro 2.10.1

@firxworx
Copy link
Author

firxworx commented Aug 3, 2023

I discovered the path issues may be even more nuanced than just --root...

I figured out a workaround for @nxtensions/astro users got Astro working for me with Nx + tailwind. It involves downgrading to astro 2.5.6 for now.

nxtensions/nxtensions#445 (comment)

I also worked around an additional postcss path related issue (covered at the link above). I'm not fully sure if its the fault of astro or the @nxtensions/astro so I will note that here in case its actually related or insightful.

In case it helps the maintainers here resolve the issue or assess impact on monorepo users, you can reference the specific flags that the @nxtensions/astro uses when running the Astro CLI in the source for its Nx "executors": https://github.com/nxtensions/nxtensions/blob/main/packages/astro/src/executors/build/executor.ts

Thanks again for all your work on Astro!

@leosvelperez
Copy link
Contributor

Curious if this might have been caused by #7647 or if this has been an issue for a while and we just never caught it before!

cc @bluwy

I traced it back to that change. It's a regression introduced in Astro v2.9.7, which is where that change was released. Up to v2.9.6, everything works fine.

From what I checked, if --root is provided (which is needed in monorepos), the final config ends up with the provided root appended twice to the process cwd and therefore, the root is invalid. This happens due to:

The issue is that root is being resolved twice and the second time, it uses the already resolved value plus the provided one. That's why we end up with an invalid path.

@bluwy
Copy link
Member

bluwy commented Aug 7, 2023

Thanks for helping tracking this down @leosvelperez @firxworx. I'll get a fix for this asap.

@firxworx
Copy link
Author

firxworx commented Aug 8, 2023

@bluwy thanks a million... honestly incredible turnaround considering Astro is an open source project! I'll try it out and advise if any issues... given the analysis + reviewing the PR for #7977 I expect this will be a winner.

@leosvelperez thanks for your thoughtful insights + dive into the issue! Being able to use Astro smoothly with Nx is huge!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
- P4: important Violate documented behavior or significantly impacts performance (priority) feat: monorepo Related to usage within a monorepo (scope)
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants