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

Build fails when outDir (other than the default) does not exist #74

Open
stavros-k opened this issue Mar 22, 2024 · 10 comments · May be fixed by #126
Open

Build fails when outDir (other than the default) does not exist #74

stavros-k opened this issue Mar 22, 2024 · 10 comments · May be fixed by #126

Comments

@stavros-k
Copy link

When I have an outDir different than the default (dist) build fails with

...
dist/build/q-Buz2BylG.js                   0.06 kB │ gzip:   0.07 kB
dist/build/q-Laqs9abf.js                   0.10 kB │ gzip:   0.11 kB
dist/build/q-C-iu1w9O.js                  45.26 kB │ gzip:  18.38 kB
dist/build/q-CFzq9JpO.js                  63.51 kB │ gzip:  20.71 kB
✓ built in 759ms
ENOENT: no such file or directory, scandir '/home/user/projects/my-project/build/'
  Stack trace:

(Yes stack trace is empty!)

If I mkdir build and run build again, it works fine.
Also if I change back the outDir to dist it also works file even if dist is missing.

Side note:
Not sure if this is expected, when I build (after the mkdir build) I can still see a dist dir created, including qwik artifacts.
I suppose those are generated there before moved to the actual outDir?
If that's the case, might be a good idea to use the cacheDir (by default node_modules/.astro) so CI's can also cache this easier

Versions:

{
    "dependencies": {
        "@astrojs/check": "0.5.9",
        "@astrojs/sitemap": "3.1.1",
        "@astrojs/tailwind": "5.1.0",
        "@builder.io/qwik": "1.5.1",
        "@fontsource-variable/raleway": "5.0.18",
        "@qwikdev/astro": "0.5.10",
        "astro": "4.5.8",
        "tailwindcss": "3.4.1",
        "typescript": "5.4.3"
    },
    "devDependencies": {
        "@types/node": "20.11.30",
        "@typescript-eslint/parser": "7.3.1",
        "astro-devtool-breakpoints": "0.3.0",
        "astro-meta-tags": "0.2.2",
        "astro-page-insight": "0.5.0",
        "eslint": "8.57.0",
        "eslint-plugin-astro": "0.32.0",
        "prettier": "3.2.5",
        "prettier-plugin-astro": "0.13.0",
        "prettier-plugin-tailwindcss": "0.5.12",
        "wrangler": "3.36.0"
    }
}

Thanks for the integration!!

@thejackshelton
Copy link
Member

Hey @stavros-k! Tried reproducing and perhaps some user error on my part 🤔 . Could you make a minimal reproduction of the issue?

@stavros-k
Copy link
Author

Okay, I've narrowed it down.

Create a new Astro project, and add qwik integration.

Create a Demo.tsx in src/components/qwik/Demo.tsx

Add a simple counter component.


This does not work

        qwikdev({
            include: "**/qwik/*",
        }),

This work

        qwikdev({
            include: "",
        }),

If you still can reproduce, let me know where I can send you a zip!

Thanks

@thejackshelton
Copy link
Member

Gonna dive in and see what's going on here in a bit 😄 .

If you'd like to take a stab at the fix, or play around with the integration, I've written a contributing guide. Let me know if anything isn't clear!

@thejackshelton
Copy link
Member

Also side note @stavros-k would suggest updating to the latest version, had a major fix to slots.

@thejackshelton
Copy link
Member

thejackshelton commented Mar 25, 2024

What I've tried so far is ensuring the directory exists with fsExtra.ensureDir(distDir)

This seems to get it to work with a static build, but not when the output is server 🤔

server error:

Cannot find module '/home/jackshelton/dev/open-source/astro/qwikdev-astro/apps/astro-demo/my-project/dist/server/manifest_lgZETu9y.mjs' imported from /home/jackshelton/dev/open-source/astro/qwikdev-astro/node_modules/.pnpm/astro@4.5.2/node_modules/astro/dist/core/build/pipeline.js
  Stack trace:
    at finalizeResolution (node:internal/modules/esm/resolve:255:11)
    at defaultResolve (node:internal/modules/esm/resolve:1121:11)
    at ModuleLoader.resolve (node:internal/modules/esm/loader:365:25)
    at ModuleLoader.import (node:internal/modules/esm/loader:328:34)
    at importModuleDynamicallyCallback (node:internal/modules/esm/utils:176:14)

@thejackshelton
Copy link
Member

thejackshelton commented Mar 25, 2024

Ok I got something working with:

6addd06

Before we can try it, we need to figure out how to setup changesets snapshot releases, because changes to the build such as this can cause regressions.

I am also hoping to have a test suite here soon that would prevent that as well.

We have an action here that was grabbed from the Astro repo for the most part.
https://github.com/QwikDev/astro/blob/main/.github/workflows/snapshot-release.yml

But doesn't seem to be working when I try to run the action, or make a new PR.

If you'd like to take a shot at getting that working that'd be awesome. Happy to figure it out in a couple days and setup a snapshot release if you can't.

@stavros-k
Copy link
Author

Hey, I'll try to test it asap and let you know!

As for the workflow not working, I think its because its "restricted" to PullRequests (

if: ${{ github.repository_owner == 'QwikDev' && github.event.issue.pull_request && (contains(github.event.comment.body, '!preview') || contains(github.event.comment.body, '/preview') || contains(github.event.comment.body, '!snapshot') || contains(github.event.comment.body, '/snapshot')) }}
)

You probably need some other || condition to also allow workflow_dispatch.
I couldn't find anything with a quick look, but you can probably print the github object and see for your self whats avaiable! (Link on how to do that)
https://github.com/orgs/community/discussions/27058#discussioncomment-3254474

@stavros-k
Copy link
Author

6addd06

That seems to work!

@siguici
Copy link
Collaborator

siguici commented Mar 27, 2024

What I've tried so far is ensuring the directory exists with fsExtra.ensureDir(distDir)

I also followed this track but I noticed that it only ensures the existence of the temporary directory and not on the output directory.

@thejackshelton
Copy link
Member

thejackshelton commented Mar 30, 2024

Hey @stavros-k. we're making a community effort to get everything setup so that we can start making safe changes to the build process, including the merging of this PR. Thanks for waiting! Here's the issue #82

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants