jsdrupal / drupal-admin-ui

This is an admin UI for Drupal, built with JavaScript & React. ⬅️✌️➡️
Other
236 stars 91 forks source link

Convert the utilities package to typescript. #750

Open martinfrances107 opened 5 years ago

martinfrances107 commented 5 years ago

The intention here is to convert the utilities package into typescript.

packages/utilities/src/api.ts is a good candidate for conversion, it is central point to contact for all interaction with the back-end. So I want the parameters to the fetch call - strongly typed, semantically clean and free of any idiosyncrasies which will lead to bugs or exploits.

a) I have use foundational types like 'RequestInit' and 'response' to lock things down. Until fixed what the linting errors showed was that we were misusing an object, passing in unexpected parameters and expecting it to be available once the object was returned. In the code look for the new variable 'isResponseText' for the alteration.

b) This line below forced the use of es7 transpilers. In other places I introduced a private class methods which are only valid in es7 [ see error.ts ]

if (![200, 201, 204].includes(res.status)

c) For *.ts files the webpack loaders have been chained together so that all the targeted browser constraints still apply.

d) As a discussion point this increases the size of the build, the output from npm run build [ in the packages/utilities directory] from 16.3KiB to 19.5Kib ( 20% increase ) I think this is small in absolute numbers, but no summary would be complete without mentioning it.

Anyway I hope this is seen as a good change.

There is a discussion to be had about the use of "d.ts" files.. If any reviewer want to ask for changes I will be very flexible and be happy to change my approach.

martinfrances107 commented 5 years ago

Now the tests are passing ..

here is my todo list

a) I think tslint is deprecated. - I think I could refactor so that eslint could be used to work with typescript b) I need to find a way to keep the tslint artifact if we are keeping tslint.