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

Page not included in build when directory of same name exists in public-directory #68

Closed
FloezeTv opened this issue Mar 11, 2024 · 2 comments

Comments

@FloezeTv
Copy link
Contributor

When a directory exists in public with the same name as a page and a qwik component exists anywhere in the project, that page is not showing up in the final build.

To reproduce

  • npm create astro@latest (Empty, with Typescript in Strict mode)
  • npx astro add @qwikdev/astro and modify tsconfig.json according to the documentation
  • Create a page (e.g., cp src/pages/index.astro src/pages/some_page.astro)
  • Create a directory in public with same name (e.g., mkdir public/some_page)
  • Create a qwik-component anywhere (e.g., a hello world component in src/component.tsx)
  • Build (npm run build) and observe that some_page is missing from the build in dist
  • The page will show up in the build again when either the directory in public is deleted or all qwik components are removed

Reason

During build, files/directories from dist are moved to a temp-directory. At this time, it seems the files from public already exists in dist but the pages have not been built yet.
Then, qwik runs its build in that temp-directory and astro builds the static pages in the dist-directory.
Qwik then moves all files/directories directly in the temp-directory back to the dist-directory.
In this case, this overwrites the generated page in the some_page-directory with the previously moved version from the public-directory.

A simple fix would be to replace fsExtra.move with fsExtra.copy, as this seems to merge the directories correctly (at the cost of having to duplicate the data).
This merging-behavior for fsExtra.move was discussed in jprichardson/node-fs-extra#668, but apparently only implemented in a separate package.

Depending on how you would want to resolve this issue, I can look into creating a pull-request if you want.

@thejackshelton
Copy link
Member

Hey @FloezeTv! I was able to reproduce the issue given your detailed steps. We've also had our fair share of weird issues with the build files being moved, especially with Astro's deployment adapters, which I wonder if this could help resolve as well.

Interesting, the package does appear to have a test suite, I wonder if it being cjs would potentially cause any issues (especially with vite's whole esm change)

A PR would be awesome 😄 . Happy to review it asap.

FloezeTv added a commit to FloezeTv/qwik-astro that referenced this issue Mar 12, 2024
Would previously overwrite all contained files
@FloezeTv
Copy link
Contributor Author

Hey @thejackshelton,
I have created a pull request. Let me know if that works or if it needs any further changes. 😃

thejackshelton added a commit that referenced this issue Mar 13, 2024
Fix #68: Make moveArtifacts merge directories
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

No branches or pull requests

2 participants