(BSR) perf(firebase): gestion des performances par firebase #7271
Conversation
pcanthelou-pass
changed the title
pcanthelou-pass
force-pushed
the
bsr/firebaseperfv1
branch
from
be0e598 to
8d18136
Compare
pcanthelou-pass
force-pushed
the
bsr/firebaseperfv1
branch
from
8d18136 to
a09b71a
Compare
|
|
There was a problem hiding this comment.
Hello, je passe juste par là, stylé que tu ais les mains encore dans le code, je te fait des retours sur les bonnes pratiques au sein de l'équipe plus que sur la partie métier.
- essaie de remplacer tous les fichiers
.tsxqui n'utilisent pas dejsxpar des fichiersts - remplace les
test('...')parit('should ...')
|
|
||
| jest.mock('shared/performance/useFirebasePerformanceProfiler', () => ({ | ||
| useFirebasePerformanceProfiler: jest.fn(), | ||
| })) |
There was a problem hiding this comment.
Il faudrait mettre ce mock dans les fichiers de tests concernés, dans le cas contraire ce mock sera chargé même dans les fichiers qui ne l'utilisent pas #6531
| // Supprimer les espaces en début et fin | ||
| let sanitized = name.trim() | ||
| // Supprimer les underscores au début et à la fin | ||
| sanitized = sanitized.replace(/(^_+)|(_+$)/g, '') | ||
| // Tronquer à 32 caractères maximum |
There was a problem hiding this comment.
les commentaires peuvent peut-être supprimés pour un naming plus parlant ou un stringBuilder pour encapsuler les implémentation de méthodes et exposer des noms parlant (overkill)
| type useFirebasePerformanceProfilerProps = { | ||
| route?: Route | ||
| shouldProfile?: boolean | ||
| } | ||
|
|
||
| export const useFirebasePerformanceProfiler = ( | ||
| traceName: string, | ||
| props?: useFirebasePerformanceProfilerProps | ||
| ) => { |
There was a problem hiding this comment.
type useFirebasePerformanceProfilerProps<T extends ScreenNames> = {
route?: UseRouteType<T>
shouldProfile?: boolean
}
export const useFirebasePerformanceProfiler = <T extends ScreenNames>(
traceName: T,
props?: useFirebasePerformanceProfilerProps<T>
) => {ce type te permettra d'avoir l'autocomplétion avec toutes les routes et de supprimer le problème avec les as
There was a problem hiding this comment.
Et bien remplacer le nom du fichier en ts car le tsx à tendance à confondre <T extends ScreenNames> avec des composants
pcanthelou-pass
added
Waiting for review
Link to JIRA ticket: https://passculture.atlassian.net/browse/PC-XXXXX
Flakiness
If I had to re-run tests in the CI due to flakiness, I add the incident on Notion
Checklist
I have:
Screenshots
delete if no UI change
Best Practices
Click to expand
!when you know that the value can’t benullorundefined).Test specific:
usertofireEvent.