Formatting and linting

[iivvaannxx]

I am not sure if this is actually needed for a not-so-big project like this one, but just throwing it as a proposal I would gladly take on. Does the project use any kind of formatting tool or linting? I feel we could use some consesus on the code style in case the project's code keeps growing, could be a good moment.

Personally, I am used to StandardJS, but I am open to other options. Prettier could be fine too, as it's very common. @thejackshelton If you are okay with it I can work on it after some discussion, if needed.

[thejackshelton]

I am not sure if this is actually needed for a not-so-big project like this one, but just throwing it as a proposal I would gladly take on. Does the project use any kind of formatting tool or linting? I feel we could use some consesus on the code style in case the project's code keeps growing, could be a good moment.

Personally, I am used to StandardJS, but I am open to other options. Prettier could be fine too, as it's very common. @thejackshelton If you are okay with it I can work on it after some discussion, if needed.

Ah I think I use prettier as a vscode extension. Is there some sort of local config you think we should have for it? Or maybe we should mention it in the docs? 🤔

[iivvaannxx]

Yeah it's usually best if you have a local config so other users that work on this repo have the same formatting and linting rules. Prettier is good to me, but if I am not wrong is only a formatter, so we would still need some other preset for linting, like Standard or any of the other popular configs.

[thejackshelton]

Yeah it's usually best if you have a local config so other users that work on this repo have the same formatting and linting rules. Prettier is good to me, but if I am not wrong is only a formatter, so we would still need some other preset for linting, like Standard or any of the other popular configs.

Ah for linting I think we're supposed to have the qwik eslint 😅 . I'll see what we can do with prettier and add that as well today.

I'm adding qwik eslint now as a peer dependency, do you want to take a stab at the local prettier config?

I tried looking at this new tool called biome that ben holmes just posted about, but they do not support astro files.

[thejackshelton]

By the way, do you ever see a bunch of type errors from the dist folder in the Qwik integration? I've been trying to figure out how to ignore that. It's like typescript is trying to read the dist file.

For example:

dist/build/q-y67EY0oH.js:1:202 - warning ts(80006): This may be converted to an async function.

[thejackshelton]

hm...

I am trying to add the eslint configuration similar to how it is for Qwik City.

If I add the following in a user project:

  "devDependencies": {
    "eslint": "^8.55.0",
    "eslint-plugin-qwik": "^1.3.0",
    "@typescript-eslint/eslint-plugin": "^6.13.2",
    "@typescript-eslint/parser": "^6.13.2",
    "@types/eslint": "^8.44.8",
  }

We can see eslint warning in the cli, but not the yellow squiggly that you can hover over in a .tsx file in Qwik City 🤔

[iivvaannxx]

Ah for linting I think we're supposed to have the qwik eslint

For Qwik components sure we need it! But for the other Typescript code ( ts files) you need other type of plugin. The one that I proposed (StandardJS) is a plug-and-play JS syntax lint configuration (it also has a TS counterpart).

By the way, do you ever see a bunch of type errors from the dist folder in the Qwik integration? I've been trying to figure out how to ignore that. It's like typescript is trying to read the dist file.

That's weird, I don't recall receiving errors on those files, could probably be for some configuration in the VSCode extension?

We can see eslint warning in the cli, but not the yellow squiggly that you can hover over in a .tsx file in Qwik City 🤔

Is the workspace settings.json properly configured to handle tsx files? Whenever I am setting up ESLint for a project that uses tsx I usually need to add a similar snippet to this one (this is currently what I have in my Astro project with the integration).

"editor.codeActionsOnSave": { "source.fixAll.eslint": true },
"eslint.options": { "extensions": [ ".js", ".cjs", ".mjs", ".jsx", ".ts", ".tsx", ".astro" ] },
"eslint.validate": [

  "javascript",
  "javascriptreact",
  "typescript",
  "typescriptreact",
  "astro",
],

And on my root .eslintrc.cjs I plug the astro-eslint-plugin and the qwik plugin (together with Standard) as follows (this is also from my current project):

module.exports = {

  // ...otherConfig
  extends: [

    'eslint:recommended',
    'standard-with-typescript',
  ],

  ignorePatterns: [ ".eslintrc.cjs" ],
  parserOptions: {

    ecmaVersion: 'latest',
    sourceType: 'module',
    tsConfigRootDir: __dirname,

    project: './tsconfig.json',
    extraFileExtensions: ['.astro']
  },

  overrides: [

    {
      files: ['*.astro'],
      parser: 'astro-eslint-parser',
      parserOptions: {

        parser: '@typescript-eslint/parser',
        extraFileExtensions: ['.astro']
      },

      rules: { // the astro rules go here }
    },

    {

      files: ['*.tsx'],
      parser: '@typescript-eslint/parser',

      extends: ['plugin:qwik/recommended'],
      parserOptions: {

        ecmaFeatures: { jsx: true },
        extraFileExtensions: ['.tsx']
      },

      rules: { // the qwik rules go here }
    },
  ]
}

Finally all the source files that ESLint is intended to run on must be inside the include (and in exclude for those which don't) of the tsconfig.json:

"include": [ "A wildcard is accepted" ]
"exclude": [

    "**/node_modules",
    "**/dist",
    "**/build",

    ".eslintrc.cjs",
]

[thejackshelton]

Hey @iivvaannxx! Sorry for the delay, I know it has been a while, I have been very focused on fixing the core issues of the integration and I think we are in a much better spot!

I'd be happy to look into a PR for this, or even pair together on it if you're free.

[iivvaannxx]

@thejackshelton Sure! I've also been a little busy so I couldn't help around with the other issues, this shouldn't take long to set up.

This weekend I'm not available, but as soon as I am I will take a look and open a PR. I'll reach out to you if I need any help!

[siguici]

Hey all,

Just caught up on the discussion, and I'm aligned with @iivvaannxx's call for adding a formatter and linter to the project. At the same time, I agree with @thejackshelton's suggestion to stick with Prettier and ESLint using the Qwik plugin, given that it's already in place in the Qwik project itself.

I also believe it would be beneficial to integrate some tests and automation scripts to bolster the overall code quality.

Given my current commitments, I may be a bit tied up, but I'll still take the initiative to create a PR for at least the formatting and linting aspects. Feel free to share your thoughts!

[iivvaannxx]

@siguici @thejackshelton As an alternative, it could be worth it taking a look into Biome. It's this newborn linter and formatter all in one which has a 95% compatibility with Prettier. I believe it also supports JSX out of the box and maybe some accessibility rules but I am not sure (it may be somewhere in the docs explained). Look at this diff:

Bad thing is that it still doesn't support Astro, but it will in the future. If you find it to be still not mature enough I also think Prettier + ESLint is a solid combination.

[siguici]

@siguici @thejackshelton As an alternative, it could be worth it taking a look into Biome. It's this newborn linter and formatter all in one which has a 95% compatibility with Prettier. I believe it also supports JSX out of the box and maybe some accessibility rules but I am not sure (it may be somewhere in the docs explained). Look at this diff:

Bad thing is that it still doesn't support Astro, but it will in the future. If you find it to be still not mature enough I also think Prettier + ESLint is a solid combination.

I always encounter problems with Biome in my personal projects, so I cannot recommend it. There are already Astro and Qwik plugins which do the job very well. Why not take advantage of them, especially since it is in development mode?

Feel free to take a look at the PR I just submitted.

[thejackshelton]

Yeah if I recall, the creator of biome is also on the Astro core team, so I do think it's a matter of time. I had looked into biome as well.

Because it doesn't work in .astro files that's definitely a blocker. For now using eslint and prettier like @siguici mentioned and then perhaps migrating to biome at some point if possible. I would be surprised if biome didn't have a migration guide.

[millette]

Yup https://github.com/biomejs/biome/discussions/1254