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

tools: Deprecating @devexperts/tools #136

Open
sutarmin opened this issue Jun 11, 2019 · 15 comments
Open

tools: Deprecating @devexperts/tools #136

sutarmin opened this issue Jun 11, 2019 · 15 comments

Comments

@sutarmin
Copy link
Contributor

This is an issue to discuss whether we need to keep maintaining @devexperts/tools package. As @raveclassic stated in other issue (comment link):

Furthermore I'd think about dropping tools at all. It's a useless layer of abstraction introduced for the sake of abstraction. Newcomers should not only learn nuances of npm/yarn build/start steps, how to use storybook, how to configure CRA, but also this package. Is there any good reason to force this abstraction with leaking details like storybook internals anyway?

My opinion is that(copying from the same issue):

  1. Webpack configs: another way we should copy them from project to project
  2. Easier maintainability of build dependencies: upgrade one time in tools instead of upgrading in every other project.
  3. A consistent approach with this abstraction across different projects makes moving developers from project to project easier.
  4. Scaffolding of new projects is easier as well.

But AFAIK all the recent projects was scaffolded without @devexperts/tools, but with just using CRA and that may be a good argument to stop maintaining tools if we have convenient approach there.

What is the approach with webpack config overloading and working with storybook in a recent projects, @raveclassic , @scink , @mankdev ? Is it just a manual copy-pasting of those custom build files?

@devex-web-frontend/team any opinions are appreciated.

@raveclassic
Copy link
Contributor

  1. I'd stick as close as possible to default create-react-app --typescript. If we have some exotic webpack configuration we should drop it and rewrite existing code to fit default cra typescript configuration.
  2. It happens not that often to maintain a separate tools package.
  3. Default cra + @storybook is that abstraction
  4. idem

@mankdev
Copy link
Contributor

mankdev commented Jul 2, 2019

I suggest to deprecate @devexperts/tools in favour of CRA. Now it is outdated. If we will need some common library with tools in future we can create new package.

@sutarmin sutarmin changed the title tools: is this package useful? Deprecating @deevexperts/tools Jul 2, 2019
@sutarmin sutarmin changed the title Deprecating @deevexperts/tools tools: Deprecating @devexperts/tools Jul 2, 2019
@sutarmin
Copy link
Contributor Author

sutarmin commented Jul 2, 2019

@mankdev @raveclassic all of our projects use @storybook modules which require configuration anyway. (.storybook directory). Also, some of our projects use stylus and adding stylus-loader requires us to use react-app-rewired or something similar. That requires configuration as well. tsconfig.json, .prettierrc, .eslintrc.json and I might forget something else. And, btw, I hate the behavior of CRA that when we have type errors, we can't use dev application. That would also require some reconfiguring.

Do you guys think it is a good idea not to have a common place for storing such configuration files, but rather maintain everything separately per-project?

@mankdev
Copy link
Contributor

mankdev commented Jul 2, 2019

Do you guys think it is a good idea not to have a common place for storing such configuration files, but rather maintain everything separately per-project?

This package @devexperts/tools is answer for your question. Because like @raveclassic said

It happens not that often to maintain a separate tools package

=(

@raveclassic
Copy link
Contributor

raveclassic commented Jul 2, 2019

all of our projects use @storybook modules which require configuration anyway. (.storybook directory).

Not all

Also, some of our projects use stylus and adding stylus-loader

We should avoid any configuration foreign to default CRA (we don't have that much styles in react-kit to free some 1-2hrs and rewrite everything to *.module.css

requires us to use react-app-rewired or something similar

Should be avoided

tsconfig.json

This is too project-specific

.prettierrc, .eslintrc.json

Should be moved to @devexperts/lint

And, btw, I hate the behavior of CRA that when we have type errors, we can't use dev application.

Yeah, this is annoying but again I'd rather leave it to end-project configuration instead

@sutarmin
Copy link
Contributor Author

sutarmin commented Jul 2, 2019

This package @devexperts/tools is answer for your question. Because like @raveclassic said

It happens not that often to maintain a separate tools package

If by "this package is answer for your question" you mean "look at it, it's ugly", I disagree with this argument. It is a question of maintainability. And here we came to the second statement which I also disagree with.

It happens not that often to maintain a separate tools package

It depends on us. To maintain is almost to "fix it when you need a fix". From that perspective, how different tools are from react-kit, for instance? If it works - you don't need to fix it. BUT. In cases when we need to update storybook or webpack (or cra, whatever) to nex major version, common tools becomes very handy. Instead of doing the same thing in different projects a whole lot of times, we can just do it once.

And btw, I'm not defending tools like I couldn't live without them. I'm just trying to consider the situation from different points. And since you both on the side of getting rid of tools, I'm taking the other one.

@raveclassic
Copy link
Contributor

webpack version is managed by cra. storybook is managed by their cli so it's a matter of updating the cli.
There's literally nothing this package (tools) can help us with. It only complicate things.

@sutarmin
Copy link
Contributor Author

sutarmin commented Sep 5, 2019

Let's continue the discussion :) We had an internal discussion and as a result of it, came to the fact that cra is too opinionated and restrictive in terms of customization. The option that was proposed during the discussion is to keep tools but make them into a bunch of files that are scaffolded into every new project to keep bootstrapping of new projects easy.

The only major requirement I can think of right now is that tools still has to be an npm-package because we need to deliver updates of build configuration to the end-projects somehow.

@raveclassic
Copy link
Contributor

The point of CRA is to avoid dependency maintenance burden. This includes not only react but also all kind of webpack plugins, loaders etc.

Here're some points I'm strongly against of:

  • maintaining a CLI
  • maintaining a documentation for that CLI
  • maintaining a consistent combination of webpack/plugins/loaders

We should think of a solution which is the most cheap to maintain/develop. Currently it's CRA despite of all that pointless restrictions. It's ok until it's possible to bypass them with configuration.

@sutarmin
Copy link
Contributor Author

sutarmin commented Sep 6, 2019

We need not the tools approach to be cheap in maintaining, but the whole build process in every project. You suggest not putting efforts on maintaining tools and use cra (maintained by others) which means:

  • we'll have to spend time for migration in every project each time cra is updated. And fight all the surprises which new version gives us in every project. For me, it looks like multiplying efforts (change in every project instead of just tools updating) instead of reducing them.
  • we'll have to share the knowledge of how to solve common cra problems and avoid common pitfalls. This looks like we have to have and maintain some kind of documentation anyway.

So it looks like roughly the same amount of work, but cra doesn't allow us to be flexible.
What about the following solution:

  1. maintain a bunch of configs (webpack, babel, etc) as just files
  2. publish them as tools package
  3. write yeoman generator that will install @devexperts/tools and create default configs in the project root which will be inheriting configs from @devexperts/tools

From the perspective of end-project, this solution is easy to maintain, extend and update. From the perspective of maintaining tools package itself it just a matter of several static config files. This idea breaks only one of your "I'm strongly against of": we have to maintain webpack plugins/loaders manually. But I can see nothing bad in changing versions monthly and checking that basic build process is still working (especially with help of CI tools).

@raveclassic
Copy link
Contributor

I do agree on the point that "webpack & co" version maintenance is a matter of resources, not possibility. However even then we have some negative feedback on such maintenance (the original reason why tools package was created).

@mankdev @dramoturg Could you share some opinions on the topic?

@raveclassic raveclassic removed this from the 1.0 milestone Sep 17, 2019
@raveclassic
Copy link
Contributor

I'm playing with platform right now and trying to add a new package. Looks like plain configuration of storybook cli is not so trivial as it seemed. A typical minimal configuration of storybook + typescript requires too much effort - overriding storybook config, overriding webpack config etc.

Let's keep this discussion open for now.

@raveclassic
Copy link
Contributor

Ok, I give up. Storybook micropackage management is hell.

@sutarmin
Copy link
Contributor Author

So, yeoman generator and a bunch of static config files is a way to go?

@raveclassic
Copy link
Contributor

Not sure :/
I'm not even sure I'm still against cli because of that dependency hell.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants