Add some type packages #2347

PolariTOON · 2023-02-03T10:14:19Z

This is kept as a draft, as this is not meant to be merged as is.
The changes are not needed to migrate to TypeScript, but should ease gradually renaming files from *.js{x} to *.ts{x}.
This also updates and adds some packages to get types on which to build upon to declare ours.

On another note, while exploring what could be done to migrate, I also faced another issue that is we are not able to import *.styl files from *.ts{x} files apparently.
This is not handled in this PR.

Demo link : https://polaritoon.github.io/cozy-ui/react/ (just to show it builds)

bundlemon · 2023-02-03T10:23:22Z

BundleMon

Unchanged files (2)

Status	Path	Size	Limits
✅	dist/cozy-ui.min.css	19.67KB	+5%
✅	dist/cozy-ui.utils.min.css	9.99KB	+5%

No change in files bundle size

Groups updated (1)

Status	Path	Size	Limits
✅	transpiled/react/**	606.77KB (+52.48KB +9.47%)	-

Final result: ✅

View report in BundleMon website ➡️

Current branch size history | Target branch size history

PolariTOON · 2023-02-03T10:36:34Z

This seems to increase to size of the build significantly. Perhaps every type packages should be in devDependencies instead of dependencies 🤔

Crash-- · 2023-02-03T10:41:00Z

package.json

@@ -175,7 +175,7 @@
 "react-pdf": "^4.0.5",
 "react-popper": "^2.2.3",
 "react-remove-scroll": "^2.4.0",
- "react-select": "^4.3.0",
+ "react-select": "^5.4.0",


To be checked because there is a few BC in the lib, no? https://react-select.com/upgrade

We may be impacted by the changes on refs since we indeed use one on ReactSelect in SelectBox to measure some height. Further investigation is needed for it. The remaining BCs look ok for us.

Quel besoin d'upgrade cette lib ?

This brings types without needing to install a @types package:

Convert to TypeScript - TypeScript types now come packaged with react-select so you no longer need to have @types/react-select installed; we no longer include Flow types

J'ai finalement retiré la montée de version de ce paquet et installé @types/react-select, ça évite d'avoir à gérer le breaking change dans cette PR, et ça évite d'avoir à mettre à jour les snapshots.

Crash-- · 2023-02-03T13:44:23Z

react/SelectBox/SelectBox.jsx

@@ -7,14 +7,16 @@ import { isIOSApp } from 'cozy-device-helper'

 import Icon from '../Icon'
 import CheckIcon from '../Icons/Check'
-import { dodgerBlue, silver, coolGrey, paleGrey } from '../palette'
+import palette from '../palette'


@PolariTOON are you sure that this is not this line that create this big impact on the bundle size?

We are already importing the palette in this way at other places and the file doesn't weight so much, thus i don't think so.

JF-Cozy

Est-ce que tu peux ajouter un peu de descriptif dans la PR pour resituer un peu le contexte de pourquoi cette PR stp ?

JF-Cozy · 2023-02-06T08:48:16Z

react/SelectBox/SelectBox.jsx

@@ -7,14 +7,16 @@ import { isIOSApp } from 'cozy-device-helper'

 import Icon from '../Icon'
 import CheckIcon from '../Icons/Check'
-import { dodgerBlue, silver, coolGrey, paleGrey } from '../palette'
+import palette from '../palette'


pourquoi ce changement ici ? Quel intérêt ?

cf. #2347 (comment)

JF-Cozy · 2023-02-06T08:48:43Z

scripts/make-icon-sprite.sh

@@ -10,4 +10,4 @@ echo "Making icon sprite, output file : ${outfile}..."
 echo $icons | xargs yarn svgstore --inline -o /tmp/icons-sprite.svg
 echo "// GENERATED FILE, DO NOT EDIT THIS FILE BY HAND" > $outfile
 echo "// Use yarn sprite to regenerate" >> $outfile
-echo "module.exports = \``cat /tmp/icons-sprite.svg`\`" >> $outfile
+echo "export default \``cat /tmp/icons-sprite.svg`\`" >> $outfile


on s'est dit y'a quelques jours qu'on n'était pas chaud pour ces modifs, pourquoi le faire à nouveau maintenant ? 🤔

Pourquoi on n'est pas chaud de faire ce changement ? Ça va dans le bon sens, non ?

@PolariTOON peux-tu retrouver la PR de référence stp ?

Cela posait des soucis il me semble au niveau des tests.
A part une écriture plus moderne, que cela apporte t-il de "mieux" ? 🤔 Y'a t-il un intérêt autre qu’esthétique ?

J'avais initialement proposé les changements ici : #2342 et on avait décidé de ne pas inclure ce commit (les commentaires ont disparu apparemment, sans doute dû au retrait du commit ?).

Effectivement ça posait un problème de test, parce que je n'avais pas fait attention que le fichier généré par make-palette.sh exporte un type de valeur bien précis : un objet, qu'on importait soit directement (import palette from '../palette') soit en destructurant (import {...} from '../palette') selon l'endroit.
Quand on utilise la syntaxe ESM pour exporter quelque chose, on doit l'importer de la même manière. Donc si on utilise un export par défaut on doit faire un import par défaut, d'où le changement dans SelectBox qui était le seul fichier à faire un import nommé jusque là.

Par rapport à l'intérêt, l'idée était juste d'avoir une base de code plus cohérente en terme de syntaxe et qu'ensuite TS puisse bien identifier les imports erronés.

scripts/make-palette.sh

JF-Cozy · 2023-02-06T08:53:49Z

docs/styleguide.config.js

+ '../react/SwipeableDrawer',
+ '../react/TextareaAutosize',
+ '../react/Toolbar',
+ '../react/Zoom'


attention il me semble que certains fichier nécessitent l’extension car plusieurs fichiers index dans le dossier

Tu as des exemples ?

Effectivement j'ai eu des soucis avec ça, mais le message d'erreur du styleguide n'est pas bien clair dont je n'ai pas identifié précisément où était le problème.
J'ai adapté ce commit pour conserver le /index partout et juste retirer l'extension, car c'était plutôt ça l'objectif à la base.

J'ai parlé trop vite. Je pensais que c'était réglé parce que je n'avais plus d'erreur, mais une fois une démo de la documentation déployée, je me suis rendu compte qu'elle était complètement cassée. Je retire le commit de cette PR, ça a l'air bien plus complexe que je ne le croyais.

JF-Cozy · 2023-02-06T08:54:38Z

package.json

@@ -175,7 +175,7 @@
 "react-pdf": "^4.0.5",
 "react-popper": "^2.2.3",
 "react-remove-scroll": "^2.4.0",
- "react-select": "^4.3.0",
+ "react-select": "^5.4.0",


Quel besoin d'upgrade cette lib ?

This mainly moves each hook, `Figure` and `FigureBlock` to a dedicated folder.

PolariTOON · 2023-02-14T14:53:27Z

This should be ready for a review (here's the demo link https://polaritoon.github.io/cozy-ui/react/ to show it builds the doc fine).
Unfortunately i didn't manage to identify the issues with the styleguide config, so i dropped the commit even if it was one of the main points of this PR. Even so, I have left some other preparatory commits that aren't need anymore right now, but that shouldn't hurt.

Crash-- reviewed Feb 3, 2023

View reviewed changes

JF-Cozy reviewed Feb 6, 2023

View reviewed changes

PolariTOON force-pushed the chore/typescript-migration-attempt branch from 8ea2d99 to 1684799 Compare February 14, 2023 11:49

PolariTOON added 5 commits February 14, 2023 12:52

chore: Use ESM syntax instead of CJS for default exports

c85fdc7

chore: Generate icon sprite and palette as TypeScript files

a618d64

chore: Lint TSX files

e31be6a

chore: Use extensionless specifiers when importing JS files

1efb445

chore: Use bare directories instead of index files in specifiers

af2facf

PolariTOON force-pushed the chore/typescript-migration-attempt branch 2 times, most recently from b54b9f7 to 691f24d Compare February 14, 2023 13:28

PolariTOON added 3 commits February 14, 2023 15:28

chore: Rename some files used in the styleguide

f016c4f

This mainly moves each hook, `Figure` and `FigureBlock` to a dedicated folder.

feat: Update packages to get types

1825377

feat: Install types packages

937419f

PolariTOON force-pushed the chore/typescript-migration-attempt branch from 691f24d to 937419f Compare February 14, 2023 14:45

PolariTOON marked this pull request as ready for review February 14, 2023 16:33

PolariTOON requested review from Merkur39, Ldoppea and zatteo as code owners February 14, 2023 16:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add some type packages #2347

Add some type packages #2347

PolariTOON commented Feb 3, 2023 •

edited

Loading

bundlemon bot commented Feb 3, 2023 •

edited

Loading

PolariTOON commented Feb 3, 2023

Crash-- Feb 3, 2023

PolariTOON Feb 3, 2023

JF-Cozy Feb 6, 2023

PolariTOON Feb 6, 2023

PolariTOON Feb 14, 2023

Crash-- Feb 3, 2023

PolariTOON Feb 3, 2023

JF-Cozy left a comment •

edited

Loading

JF-Cozy Feb 6, 2023

PolariTOON Feb 14, 2023

JF-Cozy Feb 6, 2023 •

edited

Loading

Crash-- Feb 6, 2023

JF-Cozy Feb 7, 2023

PolariTOON Feb 7, 2023

JF-Cozy Feb 6, 2023

Crash-- Feb 6, 2023

PolariTOON Feb 14, 2023

PolariTOON Feb 14, 2023

JF-Cozy Feb 6, 2023

PolariTOON commented Feb 14, 2023

Add some type packages #2347

Are you sure you want to change the base?

Add some type packages #2347

Conversation

PolariTOON commented Feb 3, 2023 • edited Loading

bundlemon bot commented Feb 3, 2023 • edited Loading

BundleMon

PolariTOON commented Feb 3, 2023

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

JF-Cozy left a comment • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JF-Cozy Feb 6, 2023 • 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

PolariTOON commented Feb 14, 2023

PolariTOON commented Feb 3, 2023 •

edited

Loading

bundlemon bot commented Feb 3, 2023 •

edited

Loading

JF-Cozy left a comment •

edited

Loading

JF-Cozy Feb 6, 2023 •

edited

Loading