feat!: Frontend infrastructure - #24
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #24 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 49 50 +1
Lines 3077 3122 +45
=========================================
+ Hits 3077 3122 +45 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
As mentioned via DM (copying here for visibility), could you ask your agent to bring test coverage back to 100%? That’s the standard we follow in this repo, since adding and maintaining these tests is relatively inexpensive compared with manually assessing whether coverage is sufficient. |
| !frontend/src/lib/ | ||
| !frontend/src/lib/** |
There was a problem hiding this comment.
This is a huge red flag that you need to re-allow these folder like this.
This is a flaw of the default .gitignore that we use which is insanely messy and needs to be cleaned up at some point.
In this case here I recommend removing everything that isn't strictly necessary and maybe contributing this back in the future
I used the following one somewhere else, maybe this is a good starting point:
# Explicitly unignore hidden files for ripgrep. Individual folders can still be ignored in the blocks below.
!.*
# macOS
.DS_Store
*.icloud
# IDEs
.idea/
.vscode/
# Python
__pycache__/
.pytest_cache/
.mypy_cache/
*.py[cod]
*$py.class
.ipynb_checkpoints
.ruff_cache/
*.egg-info/
*.egg
.pixi
# coverage
.coverage
.coverage.*
coverage.xml
/coverage
# build outputs of all kinds
build/
out/
output/
dist/
.cache/
/site
docs/_build/
# dotenv files and related
*.pem
*.env
*.env.*
# frontend
node_modules
.pnpm-debug.log*
.pnpm-store
*.tsbuildinfo
playwright-report/
test-results/| [feature.frontend.tasks] | ||
| frontend-install = "pnpm --dir frontend install --frozen-lockfile" | ||
| frontend-preview = "pnpm --dir frontend run dev" | ||
| frontend-build = { cmd = "pnpm --dir frontend run build", depends-on = [ | ||
| "frontend-install", | ||
| ] } | ||
| frontend-biome = { cmd = "pnpm --dir frontend run lint:biome", depends-on = [ | ||
| "frontend-install", | ||
| ] } | ||
| frontend-eslint = { cmd = "pnpm --dir frontend run lint:eslint", depends-on = [ | ||
| "frontend-install", | ||
| ] } |
There was a problem hiding this comment.
Bit of a personal opinion, but nesting task different task runners is an anti-pattern.
Just use pnpm directly without having to replicate every. single. script. as a pixi task
pixi run pnpm dev works. Alternatively just use pixi shell
There was a problem hiding this comment.
Thanks for the idea!
I would push back on this a little.
We get a simpler .lefthook.yaml as well as simpler CI and one developer entry point.
Most people will never need to run any pnpm commands in this repo.
In the current state, people can just look into the pixi.toml and see all relevant entry points for development.
| "format": "biome format --write .", | ||
| "format:check": "biome format .", | ||
| "lint": "pnpm run lint:biome && pnpm run lint:eslint", | ||
| "lint:check": "pnpm run lint:biome:check && pnpm run lint:eslint", | ||
| "lint:biome": "biome check --write .", | ||
| "lint:biome:check": "biome check .", | ||
| "lint:eslint": "eslint ." |
There was a problem hiding this comment.
Just use:
format:check:biome check .,format:fix:biome format --write ., andlint:eslint .
The amount of commands is overkill
| quant_ranger/_frontend/ | ||
| _site/ | ||
| / | ||
| # Legacy local sample data; frontend exports and workflows use _site. |
There was a problem hiding this comment.
nit: why is this suddenly needed? if it is indeed legacy why wasn't this line needed previously?
| @@ -0,0 +1,67 @@ | |||
| import js from '@eslint/js' | |||
There was a problem hiding this comment.
use eslint-plugin-better-tailwindcss as well:
import pluginTailwind from 'eslint-plugin-better-tailwindcss'
// ...
{
files: ['**/*.{js,jsx,ts,tsx,mdx}'],
languageOptions: {
parserOptions: {
ecmaFeatures: {
jsx: true
}
}
},
rules: {
...pluginTailwind.configs['recommended-warn'].rules,
'better-tailwindcss/enforce-consistent-class-order': 'error',
'better-tailwindcss/enforce-consistent-line-wrapping': 'off',
'better-tailwindcss/enforce-consistent-variable-syntax': 'warn',
'better-tailwindcss/no-conflicting-classes': 'error',
'better-tailwindcss/no-duplicate-classes': 'warn',
'better-tailwindcss/no-restricted-classes': 'error',
'better-tailwindcss/no-unnecessary-whitespace': 'warn',
'better-tailwindcss/no-unknown-classes': 'error'
},
plugins: {
'better-tailwindcss': pluginTailwind
},
settings: {
'better-tailwindcss': { entryPoint: '<path-to-tailwind-config>.css', rootFontSize: 16 }
}
}| import { defineConfig, globalIgnores } from 'eslint/config' | ||
| import reactHooks from 'eslint-plugin-react-hooks' | ||
| import globals from 'globals' | ||
| import tseslint from 'typescript-eslint' |
There was a problem hiding this comment.
| import tseslint from 'typescript-eslint' | |
| import { configs as javascriptConfigs } from '@eslint/js' | |
| import { configs as typescriptConfigs } from 'typescript-eslint' |
and then use:
javascriptConfigs.recommended,
typescriptConfigs.recommended,
typescriptConfigs.recommendedTypeChecked,
typescriptConfigs.stylisticTypeChecked,instead of only [js.configs.recommended, ...tseslint.configs.recommendedTypeChecked].
The change to the variable names is absolutely a nit pick, but would like to see a few more typescript eslint rules being enabled 😅
Note: The "suggestion" doesn't apply cleanly as the import from @eslint/js would be duplicated and a few more changes would be needed for everything to work 😅
| { | ||
| // These modules validate data from URL parameters and local storage before | ||
| // exposing typed values. The standard library types JSON arrays as any[]. | ||
| files: ['src/copier/dashboard-url.ts', 'src/updaters/usePullRequests.ts'], | ||
| rules: { | ||
| '@typescript-eslint/no-unsafe-argument': 'off', | ||
| '@typescript-eslint/no-unsafe-assignment': 'off', | ||
| '@typescript-eslint/no-unsafe-member-access': 'off' | ||
| } | ||
| } |
There was a problem hiding this comment.
This shouldn't be necessary. In almost all cases you don't actually have to disable eslint rules. Small changes to your types will most likely fix the issue.
| * { | ||
| box-sizing: border-box; | ||
| } | ||
|
|
||
| body { | ||
| margin: 0; | ||
| } |
There was a problem hiding this comment.
I suspect this will be done in a PR stacked on top of this one so this might not really be that applicable, but I'll mention it anyway:
Use a reset stylesheet. This is something tailwind gives you by default, so I suspect this is just an artifact of how the stacked PRs got split up.
With tailwind (or a reset stylesheet) you don't need to include this.
Changes
Introduces the shared infrastructure for static report frontends:
quant-ranger.quant-rangerfrontend export and GitHub Pages deployment documentation.