Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change Request: Make enforcement of schema optional #67

Closed
2 of 5 tasks
voxpelli opened this issue Jun 24, 2024 · 4 comments
Closed
2 of 5 tasks

Change Request: Make enforcement of schema optional #67

voxpelli opened this issue Jun 24, 2024 · 4 comments
Labels
enhancement New feature or request

Comments

@voxpelli
Copy link

Which packages would you like to change?

  • @eslint/compat
  • @eslint/config-array
  • @eslint/migrate-config
  • @eslint/object-schema

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: eslint/eslint#18619 (comment)

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 eslint/config-inspector#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 eslint/config-inspector#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

  • I am willing to submit a pull request for this change.

Additional comments

As mentioned in eslint/eslint#18619 (comment), 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: eslint/eslint#18619 (comment)

@voxpelli voxpelli added the enhancement New feature or request label Jun 24, 2024
@nzakas
Copy link
Member

nzakas commented Jun 24, 2024

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
Copy link
Author

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
Copy link
Member

nzakas commented Jun 24, 2024

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.

@nzakas nzakas closed this as not planned Won't fix, can't repro, duplicate, stale Jun 24, 2024
@voxpelli
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Archived in project
Development

No branches or pull requests

2 participants