keboola / indigo-ui

Indigo UI library, its CSS and React components. Also Styleguide app showing usages of this library.
https://indigo.keboola.com
MIT License
5 stars 0 forks source link

Eslint & Prettier integration #280

Closed natocTo closed 6 years ago

natocTo commented 6 years ago

Fixes #81

Ještě to tam trochu koliduje s ikonami co jsou generované. Respektive je chce pak eslint/prettier upravovat. Tak by se to generování mohlo podle toho připravit, nebo upravit toto pravidlo.

ujovlado commented 6 years ago

@natocTo vieme docielit aby mi to checkovalo aj ked mam zapnute yarn storybook?

ujovlado commented 6 years ago

A tento commit https://github.com/keboola/indigo-ui/pull/280/commits/0219d2e4aa9aaa874ff81746a8bee555f7781d09 by som prilepil asi ako fixup k inemu

ujovlado commented 6 years ago

Tie ikony mozeme generovat rovno validne, to nie je problem. Oni dufam casom zmiznu a prejdeme na font-awesome

ujovlado commented 6 years ago

Prehodim este 2 zmeny do mastra (nech tu toho nie je tolko) .prettierrc a upravu generovania ikoniek ... potom to rebasnem.

natocTo commented 6 years ago

@natocTo vieme docielit aby mi to checkovalo aj ked mam zapnute yarn storybook?

No toto mám integrované v Editoru. Podle těch konfigů když existujou, tak mi to kontroluje všechny otevřené záložky. Kde při uložení se mi vše samo upraví (pokud to mám zaplé). To bude i na další editory určo. Nebo myslíš něco jiného?

natocTo commented 6 years ago

A tento commit 0219d2e by som prilepil asi ako fixup k inemu

Jo to se mi zase "povedl" rebase, zkoušel jsem to třikrát nějak vrátit a udělat znovu a nepovedlo se. Tak jsem to tam nechal, že si dyštak znovu nechám poradit.

ujovlado commented 6 years ago

Pockat, tak ja to este vyskusam ... mozno som len zle nasimuloval nejaky eslint error. Ide mi ale o to, ze ked mam spustene yarn storybook, tak aby tam pisalo eslint errory (formatovat mi to samozrejme nema, to by sa mi kod menil pod rukami).

ujovlado commented 6 years ago

Napr. vyhodim z eslintrc "react/button-has-type": 0, ... spustim yarn storybook, no ziadne errory nepise. A pritom yarn eslint vypise 3.

natocTo commented 6 years ago

Jo už asi rozumím. Něco jako máme eslint-loader v KBC-UI? S create-react-app sem nedělal, tak nevím zda jde takto upravit, aby to nebylo tak že je potřeba ejectnout. Podívám se na to.

ujovlado commented 6 years ago

Jo, tam je to prave "hacknute" tak, ze to berem z react-scripts a nastavim to storybook webpack configu

ujovlado commented 6 years ago

Cize sa tam poziva eslint-config-react-app. Tak aby nenastal problem, ze jeden to bude chciet upravit inak ako druhy.

ujovlado commented 6 years ago

https://github.com/facebook/create-react-app/tree/next/packages/eslint-config-react-app

ujovlado commented 6 years ago

Neprihodime si tam aj:

{
  "extends": "react-app"
}

?

natocTo commented 6 years ago

To asi přidá/ubere pouze nějaký pravidla ne? Myslím že create-react-app bude mít nějaká "default" nastavení. Které člověk může využít nebo nemusí. Když chce svoje vytvoří si svůj eslintrc a nastaví si eslint podle svého. Aspoň tak bych čekal že to bude fungovat.

natocTo commented 6 years ago

Už vidím ten storybook config. Tak stejně jak se vyhazuje eslint config z react-scripts. Tak asi můžeme přidat náš vlastní eslint-loader. Zkusím.

ujovlado commented 6 years ago

Ok, skusim este raz. :)

Mame tam teraz 2 kongiguracie eslintu. Jedna, ktora je vytiahnuta z react-scripts (kukni webpack.config.js)

Druha je ta, co si momentalne pridal.

Chcel by som to teda zjednotit ...

ujovlado commented 6 years ago

jo, viac to pisu tu: https://storybook.js.org/configurations/custom-webpack-config/

ujovlado commented 6 years ago

Vlastne nam tam treba pridat toto:

{
        test: /\.(js|jsx|mjs)$/,
        enforce: 'pre',
        use: [
          {
            options: {
              formatter: eslintFormatter,
              eslintPath: require.resolve('eslint'),
              // @remove-on-eject-begin
              baseConfig: {
                extends: [require.resolve('eslint-config-react-app')],
              },
              ignore: false,
              useEslintrc: false,
              // @remove-on-eject-end
            },
            loader: require.resolve('eslint-loader'),
          },
        ],
        include: paths.appSrc,
      },

ale nech to bere nas eslint :)

ujovlado commented 6 years ago

No a popri tom mi napada, ci sa neoplati pozriet aj na react-app pravidla ... mozno je to celkom dobre

natocTo commented 6 years ago

Eslint-loader přidán a aplikován. Nevím zda chceme na chybu error nebo warning. V kbc-ui je error tak jsem to nechal i tu.

ujovlado commented 6 years ago

No a fici to? skus napriklad s tymi buttonmi, co som spominal.

natocTo commented 6 years ago

Jo zkusil jsem v jednom souboru odstranit uvozovku a nahlásilo mi to.

ujovlado commented 6 years ago

screenshot_2018-09-26_16-20-31

ujovlado commented 6 years ago

hore je yarn storybook, dole je yarn eslint

natocTo commented 6 years ago

Ještě to prozkoumám.

ujovlado commented 6 years ago

Tvl, uz viem cim to je ... fuck, uz som sa na tom osral 2. krat. Webpackovy eslint-loader predsa loaduje len subory, ktore su v "entry" subore.

natocTo commented 6 years ago

Aha, já se s tím chvilku pral a pak to odložil na večer. OK