Open Haprog opened 8 months ago
Regarding point 2. Running lit-analyzer
in feature/fds-navigation-mobile-version branch, the errors we have now are from rules:
no-invalid-css
margin: 0 ${FdsSize2} 0 ${FdsSize1};
(highlights second expression as the issue).
I think the issue is that combining multiple style properties (CSSResult objects) into a single CSS property value is not supported (not sure if this is just a false positive or issue in the analyzer)? I'm not sure if this is a real problem or just a limitation of lit-analyzer
but seems like we could get around this by manually combining these into a single value via unsafeCSS like this margin: ${unsafeCSS(`0 ${FdsSize2} 0 ${FdsSize1}`)};
Unknown property: 'container-type'
and Unknown at rule @container
These seem to be false positives. The issue seems to be in vscode-css-languageservice
dependency (which is used by lit-analyzer
) that is actually used to validate CSS here. We seems to be using quite old version of this so let's see if updating npm dependencies can help here.no-incompatible-property-type
Missing '{type: Boolean}' on @property decorator for 'disabled'
Missing '{type: Array}' or '{attribute: false}' on @property decorator for 'options'
no-unknown-attribute
inert
attribute in <div class="overlay" inert></div>
. This seems like a false positive as it should be a global attribute. Let's see if updating dependencies helps with this or if we can configure this to not complain about inert without disabling the rule.no-incompatible-type-binding
Type '(item: Item) => TemplateResult' is not assignable to '(item: ) => Symbol(undefined) | TemplateResult'
this is only in src/stories/fds-table.stories.ts
. Not sure what is the problem. My VS Code does not highlight this as an error. Possibly a false positive. Let's see if updating deps helps or maybe we can just configure lit-analyzer to not check src/stories/
.
CI build (
.github/workflows/ci.yml
) currently runs:This fails the build eagerly if one of the scripts exits with an error so for example prettier error in
stylecheck
will fail so that you don't even get to see possible errors fromcompile
orlint
scripts (there's a possibility that you will just see the error in CI log, fix that and push a fix only to be greeted with other/new errors when the build goes further).It would be better to run all code checks (tsc type errors, prettier, eslint, lit-analyzer) and show output from all of them in the build log before failing the build. This can be done for example with npm-run-all like so
npm-run-all --continue-on-error stylecheck compile lint
.We have
ts-lit-plugin
configured intsconfig.json
but warnings/errors from it can now only be seen with IDE plugins like lit-plugin in VS Code. We should include running lit-analyzer as part of CI build to include these warnings/errors also as part of linting step. Though there currently seems to be several unresolved issues reported by this so they need to be resolved (either by fixing or changing config) before enabling lit-analyzer in the CI build. See the output ofnpx lit-analyzer
: "33 problems in 12 files (29 errors, 4 warnings)".It would also be good to have separate npm scripts for running each code check tool individually (when manually checking specific issues locally) and also another script for running them all in one (via
npm-run-all
). E.g.lint
script could runnpm-run-all --continue-on-error lint:*
and there could belint:eslint
,lint:tsc
(should runtsc --noEmit
to only check errors),lint:prettier
,lint:lit
scripts.Make sure the script names are logical and remove possible unnecessary scripts. e.g. regarding current "build" vs "compile" I think we only need one of them and it should be enough to only run tsc if we have all code checks on "lint" script.