Update: move init command to separate package #58

anikethsaha · 2020-06-28T15:30:27Z

Summary

Move the init command to eslint-cli
Move the whole logic to the eslint-cli repo. Use the shipped shared folder from the main repo to the eslint-cli repo

Related Issues

N/A

kaicataldo

This RFC brings up an interesting point. I know the idea of splitting ESLint into multiple packages has been discussed over the years, and I do think it's worth considering whether or not that's something we would like to do:

In my mind, there are four packages in ESLint core:

CLI
Core
Core rules
eslint-config-eslint

Pros of splitting core up into multiple packages:

We can version each of these separately, which means that rule breaking changes can be released independently of breaking changes in core, which can be released independently of CLI breaking changes, etc. This could allow us to ease the upgrade path for folks, as well as not having to tie all these disparate responsibilities to each other and to one annual major release.
Maintaining each individual package could make it easier for contributors to focus on the part of the project they're most excited about contributing to.

Cons:

Coordinating releases where one package relies on the changes of another would become more burdensome. As an example, adding optional chaining support to core would have to be coordinated with the change to core rules (though this is easily fixed by release core first and then updating the rules and releasing them afterward).
General maintenance burden could be increased. Monorepos bring there own set of challenges, as do projects split across multiple repos. I think I would advocate against the monorepo approach if we decided to take this on.

I know this isn't exactly what this RFC is about, but I do see this discussion as a prerequisite to implementing an RFC like this because we should decide up front what the responsibilities of each package will be.

ilyavolodin · 2020-06-30T01:25:22Z

Not sure this is related to this RFC but...

In my mind, there are four packages in ESLint core:

CLI
Core
Core rules
eslint-config-eslint

While this separation makes sense, I think separating by functionality would be significantly more beneficial. I would separate it into:
Configuration management
Rules Core
AST Processing and Event Emitting
Core Rules
ESLint

mysticatea · 2020-06-30T10:04:23Z

Thank you for your contribution!

Actually, that's what I wanted to do (eslint/eslint-cli#4), but I didn't have the energy for it at that time. The eslint --init command local-installs eslint and configs/plugins, but it needed eslint to be local-installed. It was not convenient a bit. It's good to move --init functionality to eslint-cli.

Or, the npx command exists currently, so we can choose another solution. For example, we create create-eslint package and recommend to use npm init eslint or npx create-eslint or yarn create eslint.

anikethsaha · 2020-06-30T10:25:36Z

@kaicataldo agreed with your comment. true

Even a monorepo makes sense for me. The problems come while setting up a monorepo i.e the initial stage. About the release, I think it makes sense to go for a custom script for releasing instead of using monorepo management tool.
typescript-eslint is I think a good example to follow.

@ilyavolodin I think it that way, it will increase the number of packages, cause I can think of adding ruleTester, scope manager, config resolver as a separate package as well?

Or, the npx command exists currently, so we can choose another solution. For example, we create create-eslint package and recommend to use npm init eslint or npx create-eslint or yarn create eslint.

@mysticatea I think create-<> is generally used to create a new project right ? here, it is mostly like adding a file in an existing project.

kaicataldo · 2020-06-30T16:49:30Z

I like the idea of creating a new create-eslint command! I don’t think the current init implementation uses ESLint core at all, so separating this functionality entirely from the CLI seems like a simple way to separate out this functionality from the local vs global installation confusion. The intention could be that this tool could either be used through npx or installed globally.

nzakas · 2020-06-30T17:03:59Z

I'd definitely like to split out --init into a separate package. I'd envision something like @eslint/init, so in our instructions we could tell people to run:

npx @eslint/init

That could install the latest ESLint (if necessary) and then walk people through the regular setup. I'm 👍 to removing from the core, just not in favor of adding to eslint-cli.

anikethsaha · 2020-06-30T17:37:11Z

I am +1 for both the options (moving to eslint-cli or create a new package. )
I preferred eslint-cli as it does exist already and it is in the scope of init command. as far as the npx is concerned, it would be same for eslint-cli as well .

one con of using it in the eslint-cli can be that it is quite outdated. so a little more work to fix it.
Any other cons of considering eslint-cli ?

Else, I think create-eslint can be a better option as this has more runner support.

but as the name is concerned, it might be misleading as it can mean, create eslint plugin as well. I am thinking like create-eslint-config ?

nzakas · 2020-07-07T20:00:00Z

I'm not in favor of adding more into eslint-cli. I'm not sure how useful it really is at this point, so I'd prefer not to add more functionality into it. I'd much prefer a separate utility.

And for a separate utility, I'd prefer new packages we create use the @eslint namespace. We will already have @eslint/eslintrc after #9 is implemented, and I think that namespace gives a nice visual indicator to end-users that this is, in fact, maintained by the ESLint team.

anikethsaha · 2020-07-08T08:49:32Z

Cool,

Can you name the package. I think @eslint/create-eslint-config might be good. You guys can suggest. And accordingly, I will update the RFC

nzakas · 2020-07-09T19:44:39Z

I think we can shorten it a big to just @eslint/create-config

btmills · 2020-07-09T22:47:07Z

I also really like this idea. As far as the name goes, based on the npm docs and yarn source code (the naming permutations aren't as well documented), I summarized the options thrown out so far in the table below:

package	npm	yarn	npx
`create-eslint`	`npm init eslint`	`yarn create eslint`	`npx create-eslint`
`@eslint/create`	`npm init @eslint`	`yarn create @eslint`	`npx @eslint/create`
`@eslint/init`	No shorthand?	No shorthand?	`npx @eslint/init`
`@eslint/create-eslint-config`	`npm init @eslint/eslint-config`	`yarn create @eslint/eslint-config`	`npx @eslint/create-eslint-config`
`@eslint/create-config`	`npm init @eslint/config`	`yarn create @eslint/config`	`npx @eslint/create-config`

I prefer @eslint/create-config because:

npm init @eslint/config is almost as short as the shortest possible command, npm init eslint.
It's scoped.
It's clear about what it's going to create.
It leaves room in case we ever want to adapt the Yeoman generators as npm init @eslint/plugin and npm init @eslint/rule.

anikethsaha · 2020-07-10T07:04:17Z

Thanks @btmills for the details

@eslint/create-config is much better,

I will update the RFC soon.

mysticatea · 2020-07-10T10:54:40Z

To me, @eslint/create-config sounds like to create a new shareable config (eslint-config-xxx), as well as @eslint/create-plugin for a new plugin (eslint-plugin-xxx). That said, documentation will solve my concern.

btmills · 2020-07-10T21:07:36Z

That's a good point @mysticatea. So what if we did use @eslint/create (npm init @eslint) for end users and reserve @eslint/create-{config,plugin,rule} for developers? I suppose that npm init eslint or npm init @eslint would be the first thing a new user might try before referencing documentation, and I see you've wisely reserved the create-eslint package, which could be a thin pass-through to @eslint/create if we want both to work.

anikethsaha · 2020-07-11T06:54:12Z

@eslint/create would be more generic right? I think it would imply create plugins, config, projects?

I was thinking that @eslint/create-config for creating the config (init command). and @eslint/create-shareable-config for creating eslint-config-xxx ?

What do you think?

nzakas · 2020-07-13T19:36:34Z

I think you're expanding the scope beyond what --init does right now, so I think we should re-focus on just moving --init out of the core.

After thinking about it some more, I think @eslint/init actually makes the most sense because it does more than just create a config: it's initializing your use of ESLint in a project, including installing ESLint if it's missing, creating a config, and installing extra dependencies. So install of telling people to npm install eslint to get started, we can now tell them to run npx @eslint/init and he installation will be done for them.

We already have a Yeoman package for doing things like creating shareable configs. Maybe we could look at updating that in some way, but I wouldn't want to combine it with this RFC.

mysticatea · 2020-07-14T10:23:01Z

👍 for @eslint/init or @eslint/create.

(Off-topic: In my feeling, the most general name eslint is for end-users. And specific names such as eslint-config/eslint-plugin are for integrators.)

btmills · 2020-07-14T12:36:06Z

My concern with @eslint/init is that it only works with npx and not the npm init/yarn create shorthand that people are now used to. I’d prefer a name that works with those as well if possible.

nzakas · 2020-07-14T20:34:14Z

My concern with @eslint/init is that it only works with npx and not the npm init/yarn create shorthand that people are now used to. I’d prefer a name that works with those as well if possible.

@btmills I'm not sure I understand what you're saying here. Can you explain some more?

btmills · 2020-07-14T21:00:39Z

Can you explain some more?

Sure. In addition to npx, the package manager CLIs also paved the footpath established by create-react-app and added the npm init and yarn create shorthand commands. Now npm init react-app and yarn create react-app are equivalent to npx create-react-app, and users can use whichever command they're most accustomed to.

In the table above, I looked at a bunch of different name options and how they'd work with npx, npm init, and yarn create. Unlike the other names that have been suggested, @eslint/init would only work with npx @eslint/init and wouldn't have an equivalent npm init or yarn create shorthand. If we decide that supporting the shorthand conventions isn't important, then @eslint/init would be fine by me. My initial position is that we should support them unless there's a reason not to, so I prefer naming the package @eslint/create because it could be run as npx @eslint/create, npm init @eslint, or yarn create @eslint, whichever a user is most accustomed to.

(I initially preferred @eslint/create-config, but @mysticatea astutely observed that users might confuse npm init @eslint/config for creating a new shareable config, whereas npm init @eslint reads more like initializing ESLint for use in my project. That confusion could be exacerbated in a future world if we also had yarn create @eslint/{plugin,rule} shorthands. I agree with you @nzakas that those are out of scope for this RFC, and I brought up those names only so that we avoid using a name that we might want to use for a different purpose later.)

nzakas · 2020-07-28T18:32:24Z

Ah, I see! I actually had not been aware of this change to npm init or yarn create, I was thinking those were still just for initializing empty packages. For context for others, from the docs:

npm init [<@scope>/]<name> (same as `npx [<@scope>/]create-<name>`)

Given that, and the helpful explanation from @btmills, I support @eslint/create as the package name. 👍

anikethsaha · 2020-08-05T13:35:02Z

I have updated the change.

One question :

I dont think we can create a repo in github with name @eslint/create it will be created as -eslint-create.
So should it be created in the main repo as monorepo ?

or another option can be to create a repo name monorepo or packages and keep this package and future packages (if required) in this repo itself .

Thoughts ?

nzakas

I think the high-level direction is solid now, I'd just like to see more details about how you plan on implementing this. (See comments inline.)

designs/2020-init-command-eslint-cli/README.md

nzakas · 2020-08-05T18:28:36Z

designs/2020-init-command-eslint-cli/README.md

+    you can remove this section.
+-->
+
+Do we want to implement this as monorepo under `eslint` repo or a separate repo `@eslint/create` but github will make it as `eslint-create` or similar ?


I don't think we want to start going in the monorepo direction for this. We already have trouble dealing with eslint-config-eslint, so I'd much rather have a separate repo for this.

what should be the name of that ?

we cannot have it as @eslint/create as it will create name as -eslint-create

I don't think it's a big deal if the repo is called eslint-create. That's a pretty common convention I've seen in other projects that use namespaced package names!

kaicataldo · 2020-08-18T22:37:10Z

designs/2020-init-command-eslint-cli/README.md

+  - `shelljs`
+- create `tests/utils` and add the following functions 
+  - [`defineInMemoryFs`](https://github.com/eslint/eslint/blob/6677180495e16a02d150d0552e7e5d5f6b77fcc5/tests/_utils/in-memory-fs.js#L250)
+- for `conf/*`, it is not a part of the public API and it cant be accessed from `eslint` module, we need a keep these synced with the main repo (`eslint`). We can choose from the following approach to solve this : 


Can you elaborate on what we need this for? Is the data we need from here something we could calculate on the fly somehow? I don't think the extra calculation cost is too big a deal since this is intended to be run as a one and done script.

I'm also confused by this. I don't think --init needs anything in conf directly.

It is using it like here so I think we need to move this to the new repo right ?

May be I am missing something.

Another auto config dependency. I think we should get rid of auto config to simplify things.

nzakas

Thanks for the additional details. This really helps me understand what you're thinking. I've left a few more questions for you.

nzakas · 2020-08-20T18:28:08Z

designs/2020-init-command-eslint-cli/README.md

+  - `enquirer`
+  - `progress`
+  - `semver`
+  - `espree` (peer)


This will be brought in by including eslint, so we don't need to include it separately.

Yeah I added peer to indicate the similar.

I will remove it .

nzakas · 2020-08-20T18:29:44Z

designs/2020-init-command-eslint-cli/README.md

+  - `shelljs`
+- create `tests/utils` and add the following functions 
+  - [`defineInMemoryFs`](https://github.com/eslint/eslint/blob/6677180495e16a02d150d0552e7e5d5f6b77fcc5/tests/_utils/in-memory-fs.js#L250)
+- for `conf/*`, it is not a part of the public API and it cant be accessed from `eslint` module, we need a keep these synced with the main repo (`eslint`). We can choose from the following approach to solve this : 


I'm also confused by this. I don't think --init needs anything in conf directly.

nzakas · 2020-08-20T18:30:27Z

designs/2020-init-command-eslint-cli/README.md

+- for `conf/*`, it is not a part of the public API and it cant be accessed from `eslint` module, we need a keep these synced with the main repo (`eslint`). We can choose from the following approach to solve this : 
+  - using the `eslint-github-bot`, whenever there is a PR opened that changes the file for the `conf` directory in `eslint` repo, create an issue in the `@eslint/create` repo to have a look in the PR and if required submit a change in `@eslint/create`.
+  - OR, while doing the release for both `eslint` and `@eslint/create` repos, the release script can be configured to copy-parse the `conf` folder from `eslint` to `@eslint/create` before the release just like the `website` repo is being updated currently. 
+- After completion of this repo, whenever someone uses the `--init` flag, an error will be thrown stating the correct step to do the migration i.e using `npx @eslint/create` or `npm @eslint` or `yarn @eslint`. I am not suggesting to use run the `@eslint/create` module whenever `--init` command is being called because this would require to use the `@eslint/create` module as a dependency in `eslint` and in `@eslint/create` we are already being using `eslint` as a dependency, so there would be circular dependencies.


Why does @eslint/create need to depend directly on eslint?

because we need to use linter and cli-engine

Ref
Ref

May be as a peer ?

I’d say let’s just get rid of auto config altogether. I don’t think it’s that useful anymore. Plus, we can avoid a direct dependency on ESLint that way.

A tool like eslint-nibble will be a much better experience than autoconfig, so I have no problem with removing it 👍

Sorry but I am not getting what do you mean with auto-config ? is it the whole --init all together? or conf/* ? or something else ?

Cool, Thanks for the details.
make sense to remove autoconfig

But still, we need to use eslint as (peer)-dependency as mentioned here

Are linter and CLIEngine used outside of auto config?

no I guess, cliEngine is being used in source-code-utils but I guess if we are removing the autoconfig then we don't need that either.

Okay, sounds like we are on the right track. I'd suggest updating this RFC with the latest agreements. I think we're close!

I had such high hopes for autoconfig, but I agree that it was never terribly useful, partly because it only worked if you were already strictly following a convention. I'm actually surprised it hasn't been removed already. :)

This RFC looks like a great move. I love what y'all are doing and hope everyone is well and having a happy new year!

nzakas · 2020-08-20T18:31:43Z

designs/2020-init-command-eslint-cli/README.md

-Yes as it will be a major change for the `eslint` repo itself as it is being remove from the main package/repo, I guess a blog can be a good solution to solve inital queries. 
+Documentation for `@eslint/create` will be created with proper usage and each prompt's details. In the main documentation, a link to `@eslint/create` will be given [here](https://github.com/eslint/eslint/blob/master/docs/user-guide/command-line-interface.md#--init) and basic usage and the package details will be documented below as a deprecated notice and migration guide. 
+Also in the cli command options, [here](https://github.com/eslint/eslint/blob/master/docs/user-guide/command-line-interface.md#options), for `--init` we need to change the description for this with deprecated details as the command is still active just that there will be error that will recommend to use `@eslint/create` package instead.
+Yes as it will be a major change for the `eslint` repo itself as it is being removed from the main package/repo, I guess a blog can be a good solution to solve initial queries. 


I'm not sure that this is a breaking change. Most people only use the command once, so we're not in danger of breaking anyone's build. I'd say this is a semver-minor change.

Though I have not seen any project where it is being used in the build or in the testing so yea agreed, it is a semver-minor

Provided eslint --init automatically calls npx @eslint/create, I agree we can say this is semver-minor. I can imagine a scenario where removing autoconfig in @eslint/create breaks setup documentation someone might have published, but I don't think that rises to the level of a breaking API change.

nzakas · 2020-08-20T18:33:54Z

designs/2020-init-command-eslint-cli/README.md

-Releasing with a proper message which the command is being run from `eslint` with something like - `"this command has been moved from here, use 'npx @eslint/create'"` might be a good way to address backward compatibility. 
-Otherwise we can keep the feature in the main repo as well for starting few days/weeks with a warning and then removing it completing with an error message. 
+- Releasing with a proper message which the command is being run from `eslint` with something like - `"this command has been moved from here, use 'npx @eslint/create'"` might be a good way to address backward compatibility. 
+- Or whenever `--init` command is being used, show a warning and run `npx @eslint/create` using `child_process` of the native node modules. 


I think we can just run npx @eslint/create directly without a warning message. We'll still get the benefit of not including all of the init functionality in the main package and we'd minimize the chance of problems if someone came across some old documentation. At most we may want to just output a message like "You can also run this command directly using npm init @eslint".

I think we can make use of the output message "You can also run this command directly using npm init @eslint". ?

That works too. We should use whatever we end up documenting in the getting started guide.

anikethsaha · 2020-09-29T03:54:52Z

Sorry, for the delay. I will try to complete it by this weekend.

nzakas · 2021-01-07T01:15:42Z

@anikethsaha are you still working on this?

anikethsaha · 2021-01-07T05:40:45Z

I didnt get time for this, nor sure about whether I will get in near future.
If anyone wants to continue or wants to create a fresh RFC, feel free to do so.

nzakas · 2021-01-08T01:07:44Z

Okay, thanks for letting us know.

aladdin-add · 2021-05-03T15:34:33Z

I will take over this one. :)

nzakas · 2021-05-04T00:20:48Z

Thanks @aladdin-add!

nzakas · 2021-05-05T00:37:41Z

Closing in favor of #79

Update: move init command to eslint-cli

3464fb1

eslint-deprecated bot added the triage label Jun 28, 2020

kaicataldo reviewed Jun 30, 2020

View reviewed changes

nzakas added Initial Commenting This RFC is in the initial feedback stage and removed triage labels Jun 30, 2020

nzakas added Initial Commenting This RFC is in the initial feedback stage and removed Initial Commenting This RFC is in the initial feedback stage labels Jun 30, 2020

kaicataldo added Initial Commenting This RFC is in the initial feedback stage and removed Initial Commenting This RFC is in the initial feedback stage labels Jun 30, 2020

anikethsaha mentioned this pull request Jul 12, 2020

Roadmap Brainstorming eslint/eslint#13482

Closed

Update: changed the package name and open question

6e1ba18

nzakas reviewed Aug 5, 2020

View reviewed changes

Update: added a detail explanation for the design

35dc412

anikethsaha requested review from nzakas and kaicataldo August 17, 2020 16:21

kaicataldo reviewed Aug 18, 2020

View reviewed changes

nzakas requested changes Aug 20, 2020

View reviewed changes

mdjermanovic mentioned this pull request Jan 4, 2021

New: add yarn support to --init (fixes #13645) eslint/eslint#13756

Closed

1 task

nzakas changed the title ~~Update: move init command to eslint-cli~~ Update: move init command to separate package Mar 31, 2021

nzakas mentioned this pull request Mar 31, 2021

Reduce disk churn for ESLint upgrades eslint/eslint#14098

Closed

aladdin-add mentioned this pull request May 4, 2021

Update: separate eslint --init #79

Merged

nzakas closed this May 5, 2021

Update: move init command to separate package #58

Update: move init command to separate package #58

Conversation

anikethsaha commented Jun 28, 2020 • edited Loading

Summary

Related Issues

kaicataldo left a comment

Choose a reason for hiding this comment

ilyavolodin commented Jun 30, 2020

mysticatea commented Jun 30, 2020

anikethsaha commented Jun 30, 2020

kaicataldo commented Jun 30, 2020

nzakas commented Jun 30, 2020

anikethsaha commented Jun 30, 2020

nzakas commented Jul 7, 2020

anikethsaha commented Jul 8, 2020

nzakas commented Jul 9, 2020

btmills commented Jul 9, 2020

anikethsaha commented Jul 10, 2020

mysticatea commented Jul 10, 2020

btmills commented Jul 10, 2020

anikethsaha commented Jul 11, 2020

nzakas commented Jul 13, 2020

mysticatea commented Jul 14, 2020 • edited Loading

btmills commented Jul 14, 2020

nzakas commented Jul 14, 2020

btmills commented Jul 14, 2020

nzakas commented Jul 28, 2020

anikethsaha commented Aug 5, 2020

nzakas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nzakas left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anikethsaha Aug 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anikethsaha commented Sep 29, 2020

nzakas commented Jan 7, 2021

anikethsaha commented Jan 7, 2021

nzakas commented Jan 8, 2021

aladdin-add commented May 3, 2021

nzakas commented May 4, 2021

nzakas commented May 5, 2021

anikethsaha commented Jun 28, 2020 •

edited

Loading

mysticatea commented Jul 14, 2020 •

edited

Loading

anikethsaha Aug 23, 2020 •

edited

Loading