dso-toolkit / codestyle

0 stars 0 forks source link

Prettier #2

Open tfrijsewijk opened 6 months ago

tfrijsewijk commented 6 months ago

Voorwoord

Beste DSO-er,

In het kader van uniformiteit, onderhoudbaarheid, efficientie, en schaalbaarheid wil ik kijken of de diverse applicaties wat dichter bij elkaar kunnen komen.

Ondanks de (soms grote) verschillen is er ook overlap. Denk aan code opmaak, linting, TypeScript configuratie en browserslist. Elk team denkt na over de configuratie van deze tooling, maar we leren niet van elkaar. De kennis en beslissingen blijven binnen het team.

De voordelen zijn legio:

  • Uniforme schrijfwijze
  • Centraal updaten en toepassen van regels bij nieuwe inzichten of tool updates.
  • Kennisdeling

Met een uniforme codebase is de drempel voor developers om bij elkaar mee te kijken/helpen lager. De developer hoeft immers niet meer bekend te raken met de lokaal geldende configuratie.

In dit issue trap ik af met het centraliseren van de Prettier configuratie. Hoewel wellicht triviaal, is dit een kleine en compacte tool waarmee we dit proces kunnen doorlopen voordat we aan de grotere tooling zoals ESLint of TypeScript configuraties beginnen.

Voor dit om te slagen is het belangrijk dat we dit met z'n allen dragen. Het is niet de bedoeling dat we deze configuraties als "basis configuratie" gebruiken om lokaal verder te configureren. Smaak en kraak wordt binnen de context van de centrale configuraties betwist en besproken. Deze uitgangspositie geldt niet voor situaties waarin externe factoren de overhand hebben.

Groet, Thomas

Inleiding

Ik heb de prettier settings van alle DSO frontend applicaties geinventariseerd. Deze input heb ik naast elkaar gelegd en een gemiddelde configuratie van gemaakt.

Inventarisatie

export default {
  // toolkit
  endOfLine: "crlf",
  printWidth: 120,

  // stelselcatalogus
  printWidth: 120,
  singleQuote: true,

  // viewer
  arrowParens: 'avoid',
  bracketSameLine: true,
  endOfLine: 'auto',
  printWidth: 120,
  singleQuote: true,
  trailingComma: 'es5',

  // werkzaamheden
  bracketSameLine: true, // was jsxBracketSameLine, but that is deprecated and superseded with bracketSameLine
  printWidth: 120,
  singleQuote: true,

  // rtr frontend
  singleQuote: true,
  printWidth: 120,
  trailingComma: "none",
  arrowParens: "avoid",

  // team alpha
  arrowParens: "avoid", // graag geen parenthesis voor enkele argumenten
  bracketSameLine: false,
  printWidth: 120,  
  singleQuote: true, 
  trailingComma: 'none' //altijd een comma toevoegen vinden we niet netjes. GIT diff als argument vinden we niet sterk genoeg.
};

IHR heeft een voorkeur doorgegeven. De PR02 en PR12 applicaties hebben geen Prettier.

Gelijkwaardige configuraties

In dit overzicht zijn de applicatie configuraties aangevuld met de missende configuraties van andere applicaties met de default Prettier settings.

Hiermee zijn de configuraties gelijkwaardig aan elkaar en kunnen we een gemiddelde configuratie bepalen.

export default {
  // toolkit
  endOfLine: "crlf",
  printWidth: 120,

  arrowParens: "always",
  bracketSameLine: false,
  singleQuote: false,
  trailingComma: "all",

  // stelselcatalogus
  printWidth: 120,
  singleQuote: true,

  arrowParens: "always",
  bracketSameLine: false,
  endOfLine: "lf",
  trailingComma: "all",

  // viewer
  arrowParens: 'avoid',
  bracketSameLine: true,
  endOfLine: 'auto',
  printWidth: 120,
  singleQuote: true,
  trailingComma: 'es5',

  // werkzaamheden
  bracketSameLine: true,
  printWidth: 120,
  singleQuote: true,

  arrowParens: "always",
  endOfLine: "lf",
  trailingComma: "all",

  // rtr frontend
  singleQuote: true,
  printWidth: 120,
  trailingComma: "none",
  arrowParens: "avoid",

  endOfLine: "lf",
  bracketSameLine: false,

  // team alpha
  arrowParens: "avoid",
  bracketSameLine: false,
  printWidth: 120,  
  singleQuote: true, 
  trailingComma: 'none',

  endOfLine: "lf",
};

Gemiddelde configuratie

Onderstaand is een gemiddelde configuratie volgens meeste stemmen gelden. De configuratie arrowParens is gelijk (3 om 3).

Om het verschil te maken heb ik de default setting van Prettier meegenomen.

export default {
  arrowParens: "always",        // 3x "always", 3x "avoid". (default: "always"),
  bracketSameLine: false,       // 4x false, 2x true. (default: false),
  endOfLine: 'lf',              // 4x "lf", 1x "crlf", 1x "auto". (default: "lf")
  printWidth: 120,              // 6x 120, heeft iedereen. (default: 80)
  singleQuote: true,            // 5x "true", 1x "false". (default: false),
  trailingComma: "all"          // 3x "all", 2x "none", 1x "es5". (default: "all"),
};

Gemiddelde configuratie zonder Prettier defaults

export default {
  printWidth: 120,
  singleQuote: true
}

Impact per applicatie

In onderstaand overzicht staat IHR niet genoemd omdat er geen configuratie actief is.

Toolkit

De toolkit krijgt andere line endings (van "crlf" naar "lf") en gaat van double quotes naar single quotes.

export default {
  printWidth: 120, // ✅

  endOfLine: 'lf', // ⬅️ crlf
  singleQuote: false, // ⬅️ true
};

Stelselcatalogus

Geen impact ✅

export default {
  printWidth: 120, // ✅
  singleQuote: true, // ✅
}

Viewer

De viewer krijgt altijd parentheses (haakjes) bij arrow functions, ook als er maar één parameter (zonder type) is. Daarnaast word bij de opening HTML tag de closing angle bracket op een eigen regel gezet als de HTML attributen op een eigen regel staan.

Verder wordt er altijd een trailingComma geplaatst, en niet meer op alleen ES5 compatible plekken. Gezien de browsersupport is dat geen probleem.

En de line endings gaan van "auto" naar "lf".

export default {
  printWidth: 120, // ✅
  singleQuote: true, // ✅

  arrowParens: 'always', // ⬅️ 'avoid'
  bracketSameLine: false, // ⬅️ true
  endOfLine: 'lf', // ⬅️ 'auto'
  trailingComma: 'all', // ⬅️ 'es5'
};

Werkzaamheden

Bij de werkzaamheden applicatie wordt bij de opening HTML tag de closing angle bracket op een eigen regel gezet als de HTML attributen op een eigen regel staan.

export default {
  printWidth: 120, // ✅
  singleQuote: true, // ✅

  bracketSameLine: false, // ⬅️ true
};

RTR frontend

De RTR frontend gaat overal trailing comma's krijgen en de arrow functions krijgen altijd haakjes, ook als er maar één parameter is.

export default {
  singleQuote: true, // ✅
  printWidth: 120, // ✅

  trailingComma: 'all', // ⬅️ 'none'
  arrowParens: 'always', // ⬅️ 'avoid'
};

Participerende teams

iterox commented 6 months ago

Ik vind dit een helder voorstel 👍

hansgrimm commented 6 months ago

^^ idem

Hendriksie commented 5 months ago

Klinkt goed!

micheldebree commented 5 months ago

Prima initiatief. Ik zou deze configuratie over willen nemen als deze goed stabiel is, zodat de formattering van onze code base maar 1 keer hoeft te veranderen. (betreft RTR frontend)

tfrijsewijk commented 5 months ago

Ik zou deze configuratie over willen nemen als deze goed stabiel is, zodat de formattering van onze code base maar 1 keer hoeft te veranderen. (betreft RTR frontend)

  • Is dit de definitieve versie die voorgesteld wordt? [...]

Ik heb een inventarisatie gedaan van alle Prettier configuraties en een gemiddelde gemaakt (meeste stemmen gelden). Daar komt de volgende configuratie uit (zie openingspost, kopje "Gemiddelde configuratie zonder Prettier defaults"):

export default {
  printWidth: 120,
  singleQuote: true
}

Daarna (maandag en gisteren) heb ik bij de teams een rondje gedaan. Om iedereen een gelijkwaardige positie te geven en mee te laten beslissen in de uiteindelijke configuratie ga ik pas een versie uitbrengen als er door de participerende teams commitment/instemming is afgegeven. Mocht iemand zwaarwegende voorkeur voor een bepaalde configuratie/stijl hebben, nu is het moment om het daarover te hebben.

We slaan de plank mis als we in deze vroege fase al voorsorteren op lokale uitzonderingen.

  • [...] Op welke manier wordt die gecentraliseerd?

Voor zover ik weet is de toolkit het enige DSO component wat met NPM packages werkt. Het lijkt mij daarom logisch dat Team Toolkit de infrastructuur faciliteert en het proces begeleid. Het aandeel van de betrokkenen is gelijk. De codestyle configuraties worden geen onderdeel van de "DSO Toolkit de componenten bibliotheek".

Mochten we dit bij de toolkit beleggen zie ik voor me dat de codestyle configuraties in deze GitHub repository (github.com/dso-toolkit/codestyle, het issue wat je nu leest zit daarin) worden ondergebracht in een monorepo. Builden gaat in GitHub Actions en deployen gaat naar het NPM register onder de @dso-toolkit scope. Hiermee is elke stap (issue management, rationalisatie, build, deploy) transparant/open source.

De eerste package die met bovenstaand stramien zal verschijnen is dan @dso-toolkit/prettier-config@. Dit zal dan meteen versie 1.0.0 zijn.

  • Er zijn 4 configuraties genoemd, het lijkt me handig om er 1 te adviseren.

In dat geval heb ik mijn oorspronkelijke issue omschrijving niet duidelijk genoeg gemaakt. Ik heb hierboven al een beetje uitgelegd hoe ik te werk ben gegaan, maar het komt erop neer dat ik een

  1. inventarisatie heb gemaakt
  2. de overige configuraties heb aangevuld
  3. een gemiddelde heb gemaakt (meeste stemmen gelden)
  4. de Prettier defaults achterwege gelaten.

Mijn voorkeur gaat uit naar de meest complete (nr. 3, gemiddelde configuratie) [...]

De gemiddelde configuratie is "het eerlijkst" en heb ik tot nu toe als vertrekpunt bij de overige teams gepitched. Nu ik mijn aanvliegroute heb toegelicht, is dit voor jullie acceptabel? Wat denken jullie van de gemiddelde configuratie?

[...] zodat we minder last zullen hebben van voortschrijdend inzicht door Prettier zelf.

Prettier 3.1.1 heeft op dit moment 24 options. Ik pleit ervoor om alleen de afwijkingen in de configuratie vast te leggen. Hiermee is het in één oogopslag duidelijk waar de verschillen zitten.

Voortschrijdende inzichten van tooling zoals Prettier wordt afgevangen met:

  1. De actieve configuratie is een optelsom van de configuratie versie + Prettier versie (bijvoorbeeld @dso-toolkit/prettier-config@1.0.0 + prettier@3.1.1).
  2. De configuratiepackages geven ook aan voor welke versie van de tooling deze is gemaakt (dmv. peerDependencies in de package.json).
  3. De tooling werkt met (een vorm van) SemVer.

"Zomaar" zal er niets iets veranderen. En hoe minder we zelf doen maar op bestaande functionaliteit, hoe minder complex de opzet is en hoe makkelijker het onderhoud.

  • Ik zou ook graag overgaan als er genoeg teams zijn die dit ook doen, en niet de enige zijn :) Wordt dit ergens bijgehouden? In dit issue bijv.

Goed punt! Ik zal het eerste issue bijwerken en actueel houden, maar bij deze alvast een sneakpreview: Ik heb het idee bij Team Toolkit, de Stelselcatalogus, de Viewer, PR13, PR12 en PR02 gepitched. Ik ben met veel enthousiasme onthaald en heb al toezeggingen (in verschillende mate) gekregen van de eerste 4 teams. 3 doen er zeker weten mee.

Maar het lijkt erop dat we een vliegende start maken :)

kad-orhung commented 5 months ago

Hoi, vanuit team Alpha(IHR) hebben we de volgende voorkeuren opgesteld:```

export default {
  arrowParens: "avoid", // graag geen parenthesis voor enkele argumenten
  bracketSameLine: false,
  printWidth: 120,  
  singleQuote: true, 
  trailingComma: 'none' //altijd een comma toevoegen vinden we niet netjes. GIT diff als argument vinden we niet sterk genoeg.
}
tfrijsewijk commented 5 months ago

@kad-orhung: Dank! Ik heb jullie voorkeuren in het voorstel meegenomen.

@petervannes en @yktoo, hebben jullie hier al over kunnen filosoferen?

yktoo commented 5 months ago

@tfrijsewijk Ja, Samir gaat hierop reageren.