Skip to content

feat: Add eslint/core package#68

Merged
mdjermanovic merged 11 commits intomainfrom
core
Jun 26, 2024
Merged

feat: Add eslint/core package#68
mdjermanovic merged 11 commits intomainfrom
core

Conversation

@nzakas
Copy link
Copy Markdown
Member

@nzakas nzakas commented Jun 24, 2024

Prerequisites checklist

What is the purpose of this pull request?

This package creates the @eslint/core package, which is the future home of the runtime-agnostic rewritten core of ESLint.

Right now it just exports types for language plugins.

What changes did you make? (Give an overview)

  • Created the packaged
  • Updated the release-please script
  • Created type definitions

Related Issues

fixes #62

Is there anything you'd like reviewers to focus on?

I wasn't sure if we should be testing the type definitions somehow? I'd defer to someone with more TypeScript knowledge than I on how that might be done.

Comment thread packages/core/jsr.json
Comment thread packages/core/README.md Outdated
Comment thread packages/core/package.json
Comment thread packages/core/src/types.ts
Comment thread packages/core/src/types.ts Outdated
Comment thread packages/core/src/types.ts
Comment thread packages/core/src/types.ts Outdated
Comment thread packages/core/package.json
Comment thread packages/core/src/types.ts Outdated
Comment thread packages/core/src/types.ts
Comment thread packages/core/src/types.ts
Comment thread packages/core/src/types.ts Outdated
Comment thread packages/core/src/types.ts Outdated
Comment thread packages/core/src/types.ts Outdated
Comment thread packages/core/src/types.ts Outdated
Comment thread packages/core/LICENSE
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Docs] Following #52, it'd be good to have a decision doc for licensing. Even if it's just "we need / want to use the same as existing ESLint licenses".

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There was no decision made. The OpenJS Foundation says any new projects must be licensed under Apache 2.0.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a source that can be linked?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linked from where?

And I have no idea. We've been part of the foundation for a long time and this was communicated early on.

There might be something somewhere in https://github.com/openjs-foundation if you feel like going on a search.

Copy link
Copy Markdown
Contributor

@JoshuaKGoldberg JoshuaKGoldberg Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Linked from where?

Proposal: a decision doc, for visibility?

Copy link
Copy Markdown
Contributor

@JoshuaKGoldberg JoshuaKGoldberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(posting some quick comment-questions in lieu of a full review)

Comment thread packages/core/README.md Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following #52 and #73: I don't feel able to review this file without understanding how this is meant to play with @types/eslint over time. Is it intentionally different? Have the tradeoffs been discussed somewhere?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We weren't consulted on the creation of @types/eslint, so I consider those to be non-canonical types. As we move forward, we'll be creating types the way we want and using those. Maybe @types/eslint will get smart and start importing our types to flesh out the package, but either way, I'm not going to worry too much about it.

Copy link
Copy Markdown
Contributor

@JoshuaKGoldberg JoshuaKGoldberg Jun 26, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-canonical types

Practically speaking, those and typescript-eslint are canonical types, as those are what the entire community uses at the moment. If the decision is to explicitly not attempt backwards compat, sure, that's a reasonable decision - but I don't think they can be ignored in decision-making summarily.

Co-authored-by: Josh Goldberg ✨ <[email protected]>
Copy link
Copy Markdown
Member

@mdjermanovic mdjermanovic left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

@mdjermanovic mdjermanovic merged commit e3d309d into main Jun 26, 2024
@mdjermanovic mdjermanovic deleted the core branch June 26, 2024 16:05
@github-actions github-actions Bot mentioned this pull request Jun 26, 2024
snitin315 pushed a commit that referenced this pull request Mar 29, 2026
Co-authored-by: Milos Djermanovic <[email protected]>
Co-authored-by: Josh Goldberg ✨ <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Change Request: eslint/core as types-only package (to start)

4 participants