eslint / rewrite

Monorepo for the new version of ESLint
Apache License 2.0
65 stars 4 forks source link

Change Request: Make enforcement of schema optional #67

Closed voxpelli closed 6 days ago

voxpelli commented 1 week ago

Which packages would you like to change?

What problem do you want to solve?

One challenge when making use of @eslint/config-array to consume ESLint Flat Configs from a third party module is that the ESLint Flat Config schema is not published: https://github.com/eslint/eslint/issues/18619#issuecomment-2183187174

This makes @eslint/config-array throw due to unexpected properties in the config objects.

To avoid this I opted for making a mostly non-enforcing minimal schema in https://github.com/eslint/config-inspector/pull/69:

const noopSchema = {
  merge: 'replace',
  validate() {},
}

const flatConfigNoopSchema = {
  settings: noopSchema,
  linterOptions: noopSchema,
  language: noopSchema,
  languageOptions: noopSchema,
  processor: noopSchema,
  plugins: noopSchema,
  rules: noopSchema,
}

What do you think is the correct solution?

Instead of trying to mimic a non-published schema I would like to accept all schemas in a case like https://github.com/eslint/config-inspector/pull/69

Pretty much a:

  const configArray = new ConfigArray(configs, {
    basePath,
    schema: acceptEverything,
  })

As the current flatConfigNoopSchema will fail if the actual ESLint Flat Config schema is extended with an additional top level key.

Participation

Additional comments

As mentioned in https://github.com/eslint/eslint/issues/18619#issuecomment-2183187174, an alternative in this specific case is for sure to publish the ESLint Flat Config schema so that the @eslint/config-inspector can consume it, but since eg. @eslint/config-inspector is only interested in the files and ignores it does not need to validate the config schemas and could in theory at its core be made to support more types of config arrays than ESLint Flat Config.

And the fact that the ESLint Flat Config schema is not published is indicative that there are cases where the config schema will not be published but consumption by a third party through @eslint/config-inspector is still desired.

Another alternative that was considered was to consume the ESLint FlatConfigArray instead, but that would expose too much of the ESLint internals and be inflexible for ESLint: https://github.com/eslint/eslint/issues/18619#issuecomment-2182931487

nzakas commented 6 days ago

Is there a reason something like this won't work?

const acceptEverything = new Proxy({}, {
  get() {
    return {
      merge: 'replace',
      validate() { },
    }
  },
});

const configArray = new ConfigArray(configs, {
  basePath,
  schema: acceptEverything,
})
voxpelli commented 6 days ago

Is there a reason why something like that can not be given through a shorthand? :)

  const configArray = new ConfigArray(configs, {
    basePath,
    noSchemaEnforcement: true,
  })

The intent from both the perspective of @eslint/config-array and the perspective of eg @eslint/config-inspector becomes clearer then I think.

I would opt for the flatConfigNoopSchema over such a Proxy, as I find the former more readable than the latter.

nzakas commented 6 days ago

Is there a reason why something like that can not be given through a shorthand? :)

Yes, because this is not an intended use of the utility. The whole point of this package is to enforce a schema, so it doesn't make sense to add an option telling it not to. It doesn't make sense to provide a shorthand for something that is an exception case, especially when there is a user-land solution.

voxpelli commented 6 days ago

@nzakas Could we maybe get an official published Eslint Flat Config schema then? So that one can use the correct schema?