Conversation
lmuntaner
left a comment
lmuntaner
left a comment
There was a problem hiding this comment.
Looking super good, thanks!
There was a problem hiding this comment.
Is your IDE adding the empty spaces? Coul we revert it? I'm not sure mine is doing it. Or is it the new linter?
There was a problem hiding this comment.
It's the prettier update, since I'm updating the formatting of all files in the next PR, I don't think it's worth reverting these files manually one by one 😅
There was a problem hiding this comment.
Can we revert the changes here?
| "build": "tsc --noEmit && vite build", | ||
| "check": "tsc --project ./tsconfig.all.json --noEmit", | ||
| "check:showcase": "astro check --root ./src/showcase --tsconfig ./tsconfig.all.json", | ||
| "check:showcase": "astro check --root ./src/showcase --tsconfig ../../tsconfig.all.json", |
There was a problem hiding this comment.
Is the relative directory correct? Why did this change?
There was a problem hiding this comment.
It's correct, no idea why this worked previously without setting the relative directory correctly.
| "format": "prettier --write src/showcase src/frontend src/vc-api src/sig-verifier-js src/vite-plugins tsconfig.json tsconfig.all.json .eslintrc.json vite.config.ts vitest.config.ts demos", | ||
| "format-check": "prettier --check src/showcase src/frontend src/vc-api src/sig-verifier-js src/vite-plugins tsconfig.json tsconfig.all.json .eslintrc.json vite.config.ts vitest.config.ts demos" | ||
| "format": "prettier --write src/showcase src/frontend src/vc-api src/sig-verifier-js src/vite-plugins tsconfig.json tsconfig.all.json eslint.config.js vite.config.ts vitest.config.ts demos", | ||
| "format-check": "exit 0" |
| } | ||
|
|
||
| const logs: string[] = []; | ||
| const logit = (value: string) => { |
| @@ -1,5 +1,5 @@ | |||
| declare namespace globalThis { | |||
| any; | |||
| void any; | |||
There was a problem hiding this comment.
Yes, else eslint will give an unused error.
Overall, I also find the _unusedVariable eslint rule a bit strange, it's JS not Rust, the void operator is particularly for the purpose to indicate you don't want to use a variable. But I left that rule for now since it's a bigger discussion and would be bigger change for another PR.
| "vite": "^6.0.0", | ||
| "undici": "*", | ||
| "vite-plugin-compression": "^0.5.1", | ||
| "vite-plugin-compression2": "^1.3.3", |
There was a problem hiding this comment.
Why changing the library?
There was a problem hiding this comment.
Old library doesn't support Vite v6 and is no longer maintained, this new library does support v6.
| "moduleResolution": "bundler", | ||
| "resolveJsonModule": true, | ||
| "isolatedModules": true, | ||
| "verbatimModuleSyntax": false, |
There was a problem hiding this comment.
Yes, else it will default to requiring the import type X from "x" syntax for type imports. I think this syntax is an improvement, but not something we should do in this PR.
There was a problem hiding this comment.
So, you will revert this in this PR?
There was a problem hiding this comment.
I remember getting an error if this wasn't set to false, checking again now.
| sequence: { | ||
| concurrent: false, | ||
| }, | ||
| fileParallelism: false, |
There was a problem hiding this comment.
Can you add this comment in the code and mention the error we had in CI to remember in the future?
2 participants
Changes
theninstead)🟡 Some screens were changed