search

devexperts / dx-platform

Mozilla Public License 2.0
33 stars 24 forks source link

tools: Deprecating @devexperts/tools #136

Open sutarmin opened 5 years ago

sutarmin commented 5 years ago

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 commented 5 years ago
  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 commented 5 years ago

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 commented 5 years ago

@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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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:

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 commented 5 years ago

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:

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 commented 5 years ago

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 commented 5 years ago

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 commented 5 years ago

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

sutarmin commented 5 years ago

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

raveclassic commented 5 years ago

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