Formatting and Linting Integration with Qwik and Astro Plugins (#43)

[siguici]

Implemented formatting and linting using Astro and Qwik plugins to address style issues in the project (#43). Also, added a corresponding workflow. Future enhancements may include integrating test actions.

Your quick review is appreciated!

[thejackshelton]

Hey @siguici, awesome job!

Some things to mention, I see qwik city as a dev dependency in the root package, which should be removed since we are using Astro as an alternative.

Another thing, it looks like the build time of Qwik Astro applications has doubled, and I am not sure why

Here is the build time back in the main branch. It consistently seems to be 1-2 seconds. I'm not sure why linting and formatting in the root project would affect the build time of the demo consumer project 🤔 . Is this common?

10:18:00 [build] Building server entrypoints...
10:18:03 [build] ✓ Completed in 2.72s.

[thejackshelton]

I'm also curious on why we might need these dependencies. Would love to hear your insight there.

    "acorn": "^8.11.2"
    "vite-tsconfig-paths": "^4.2.1"
    "undici": "^6.5.0"

could these possibly be slowing the build down?

[thejackshelton]

I found what the main slowdown was. eslint-plugin-qwik 😭 . I think we need this though. I guess it just took longer because it had to lint the qwik files when building? 🤔

[iivvaannxx]

@thejackshelton I am not sure about the others but ts-config-paths was one of the deps I added myself with #38 because if not Rollup was not able to resolve path aliases inside Qwik components.

I can't test the build speed myself until Monday but I would be surprised if an ESLint plugin is slowing down the build.

Isn't the build a process separate from the linting one?

[siguici]

I'm also curious on why we might need these dependencies. Would love to hear your insight there.

    "acorn": "^8.11.2"
    "vite-tsconfig-paths": "^4.2.1"
    "undici": "^6.5.0"

could these possibly be slowing the build down?

Certainly, while executing the workflow, I found myself pondering the same question. Initially, I included dependencies for integrating both unit and end-to-end tests. However, to avoid an overwhelming PR with extensive modifications, I decided to remove some. It seems I may have missed a few in the process. I'll rectify this by cleaning up the code and making the necessary commits.

I appreciate your valuable feedback and thoughtful comments. Thank you.

[siguici]

I've removed unnecessary dependencies located at the root of the project and introduced Knip, a tool designed for detecting redundant code and dependencies.

Despite these optimizations, the execution speed remains a bit sluggish, and there's still ongoing cleanup work. To observe this, feel free to run pnpm knip. I refrained from executing it myself to avoid any unintended disruptions in other functionalities.

[thejackshelton]

Awesome! @siguici in the meantime I've reached out to the core team to ask if the increased build time is normal, and the response back was that it wasn't, so I'll continue to play around with this and see what we can do to merge it 😄

[siguici]

I understand, and I've been actively searching for the root cause of the issue, but haven't found a conclusive solution yet. I'm eager to receive a positive outcome soon.

Feel free to share any additional information or suggestions you may have. We're working together to address this issue promptly. Thank you for your understanding.

[thejackshelton]

Hey @siguici @iivvaannxx, so I haven't been able to make much progress on figuring out why eslint is slowing down the build so much.

In the meantime, I've refactored the integration to use the Astro integration kit. https://astro-integration-kit.netlify.app/. Haven't released a new version yet.

It looks like biome has high priority for astro files, since their own website is using .astro. Do you think we should wait for biome support? Or should I merge what we currently have with main?

I am happy with either solution 😄

cc @millette

[iivvaannxx]

@thejackshelton I’ve also tried to look into it without any success at the moment, I don’t have much time these days…

Personally I would keep this open either until we find out why the build time is being affected or until biome is ready to go, the one that happens first. I would like to take a deeper look but I am a bit busy and I can’t right now. Although if you decide to merge I will also be happy with it.

Probably could be worth testing if it affects the build time of other actual real projects and not the one on the repo (if not already done).

[thejackshelton]

Looks like biome added support to Astro files. Anyone want to give a shot at adding this in with the other awesome tooling @siguici has added in this pr?

I am currently working on making the view transition experience with Qwik and Astro more seamless, if I have some time this week will finish adding in the changesets and hopefully some time to finally incorporate the effort @siguici has put in here (if nobody gets to this by the time I can).

Also happy to chat over voice any concerns, questions, ideas, etc. 😄

[iivvaannxx]

@thejackshelton I've managed to use Biome in a couple of my projects already, and so far the experience is great! I see the support is only for the frontmatter of .astro files, but that should the trick at the moment, I think I can do it myself this evening. Now, I agree that @siguici added great tooling but Biome is intended to replace them all on its own, both Prettier and ESLint together with the plugins, because it does the same job. I'll make a new PR as soon as I have it.

[thejackshelton]

Closed in favor of #71