feat: Add .d.ts files generation on cozy-stack-client #1234
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Ldoppea
requested review from
Crash--,
trollepierre,
Merkur39 and
paultranvan
as code owners
Crash--
reviewed
Sep 1, 2022
| @@ -516,7 +516,7 @@ class OAuthClient extends CozyStackClient { | |||
| /** | |||
| * Updates the client's stored token | |||
| * | |||
| * @param {string} token = null The new token to use — can be a string, a json object or an AccessToken instance. | |||
| * @param {AccessToken|string} token = null The new token to use — can be a string, a json object or an AccessToken instance. | |||
Crash--
reviewed
Sep 1, 2022
| @@ -21,6 +21,10 @@ export function isSecureProtocol(urlArg) { | |||
| export function getBrowserCozyURL() { | |||
| try { | |||
| const root = document.querySelector('[role=application]') | |||
| // @ts-ignore | |||
There was a problem hiding this comment.
The ts-ignore is only for this block of code, right?
There was a problem hiding this comment.
yes @ts-ignore is for the following line. For the entire document we would use @ts-nocheck
Crash--
reviewed
Sep 1, 2022
| @@ -15,6 +15,7 @@ class ShortcutsCollection extends DocumentCollection { | |||
| * @param {string} attributes.name Filename | |||
| * @param {string} attributes.url Shortcut's URL | |||
| * @param {string} attributes.dir_id dir_id where to create the shortcut | |||
| * @param {string} attributes.type TODO | |||
There was a problem hiding this comment.
Should only be empty or io.cozy.files.shortcuts
Crash--
reviewed
Sep 1, 2022
| @@ -16,7 +16,10 @@ const headersFromString = headerString => { | |||
| * @private | |||
| * @param {string} fullpath - Route path | |||
| * @param {object} options - Fetch options | |||
| * @param {Function} options.onUploadProgress - Callback to receive upload progress events | |||
| * @param {(this: XMLHttpRequestUpload, ev: ProgressEvent<XMLHttpRequestEventTarget>) => any} options.onUploadProgress - Callback to receive upload progress events | |||
| * @param {string} options.method - TODO | |||
There was a problem hiding this comment.
I don't know if TS comes with built-in type for this, but can be one of those https://developer.mozilla.org/en-US/docs/Web/HTTP/Methods
|
Nice! (cc @acezard since we talk about that recently ;)) |
Crash--
force-pushed
the
feat/cozy-stack-client-typedefs
branch
from
9e1a551 to
928529d
Compare
Crash--
force-pushed
the
feat/cozy-stack-client-typedefs
branch
from
f72d1cf to
eb46bf2
Compare
|
I can't approve my own PR, but latest changes looks good to me 👍 |
With previous order `this.token` would be inferred as `string|AppToken|AccessToken` instead of `AppToken|AccessToken`
Crash--
force-pushed
the
feat/cozy-stack-client-typedefs
branch
from
7f79c72 to
843fce9
Compare
These types are needed in the native app. For now I did not find how to make JSDoc understand that CozyStackClient will inherit OAuthClient methods, that's the only thing not working for 100% flagship app compat
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
On our Flagship mobile app I need type declaration for
client.getStackClient()so I'm trying to add.d.tsfiles generation on cozy-stack-client like it is already set for cozy-clientThere are a lot of TS errors, so this PR may take a bit of time. It is draft for now, so you can help if you want to.
For now:
chorecommits are only JSDoc related ones, so they should not impact the lib's behaviourfeatcommits are modifying the code, so the behaviour may be impacted, we should keep our attention on themfixcommits are for fixing bugs highlighted by TS-check. My understanding is that current code should not work by construction