feat: replace custom plugin with ux consistency #3778
Conversation
|
| export default defineConfig([ | ||
| { | ||
| files: ["**/manifest.json"], |
There was a problem hiding this comment.
The manifest.json must be located in webapp - any file named manifest.json but located elsewhere should be irrelevant. Thus, this should read
| files: ["**/manifest.json"], | |
| files: ["webapp/manifest.json"], |
There was a problem hiding this comment.
That's not correct. The name of the webapp folder is defined in the UI5 yaml configuration and can be any name. /webapp is just the default (see https://ui5.github.io/cli/stable/pages/Configuration/#available-path-mappings).
That's why everybody should use the getWebappPath method from @sap-ux/project-access instead of hard code to webapp.
There was a problem hiding this comment.
Ok, I had asked @hitesh-parmar about this. Anyway, we need to tell in the eslint plugin which files the language is for - just defining all files named manifest.json regardless where they are located cannot be correct. @heimwege Should we set ui5.yaml, and then start reading everything from there? Or could this also be wrong?
@heimwege
| export default defineConfig([ | ||
| { | ||
| files: ["**/manifest.json"], | ||
| plugins: { fiori }, |
There was a problem hiding this comment.
According to the naming convention from eslint for plugins and their packages and node-modules, this should read
| plugins: { fiori }, | |
| plugins: { @sap-ux/fiori-tools }, |
However, I'm not sure about the import statement in line 29 - maybe this allows to use a different name
There was a problem hiding this comment.
You can import the plugin with any name, then you need to refer to it in the config definition (in extends, language used from the plugin). I guess you propose to change the package name?
| import { defineConfig } from "eslint/config"; | ||
| import fiori from "@sap-ux/eslint-plugin-fiori-tools"; | ||
| export default defineConfig([ |
There was a problem hiding this comment.
We should not ask consumers to import and call defineConfig in a specific way, but rather do this ourselves and provide a ready-to-use configuration (like defaultTS, defaultJS,...)
There was a problem hiding this comment.
If consumer wants to use the plugin they need eslint and an eslint config file. The config exported from the plugin is called "manifest" used in line 35 and defined here:
open-ux-tools/packages/eslint-plugin-fiori-tools/src/index.ts
Lines 19 to 22 in a290393
There can be multiple configs added in the future, also consumer can ignore it and defined a set of rules in a projects' eslint config themselves.
There was a problem hiding this comment.
We should provide configurations in a way to make it as simple as possible to use them as they are for applications. If we do that correct, applications eslint.config.mjs can look as simple as
export default config from "@sap-ux/eslint-plugin-fiori-tools/<config>";
(where should be the relevant name, depending on whether the app is being delivered from SAP, part of S/4 or own development from a customer).
This also allows apps to import the configuration, extend it, and override anything they want to - but we should not enforce this to be the standard case!
|
…ori-tools-ux-consistency
Initial changes