You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@cordova.apache.org by GitBox <gi...@apache.org> on 2022/08/17 04:10:59 UTC
[GitHub] [cordova-plugin-file] peitschie opened a new pull request, #536: fix(types): Adjust types to fix incompatibilities with TypeScript 4.4+
peitschie opened a new pull request, #536:
URL: https://github.com/apache/cordova-plugin-file/pull/536
### Platforms affected
Typescript type definitions only. No platform impacts.
### Motivation and Context
Changes in libdom by the TypeScript project led to conflicting definitions with this plugin. This is an attempt following on from #500 to try and address the compatibility problem.
### Description
The key point that joins libdom and this plugin together are via the `root` property
on the FileSystem interface. This results in the DirectoryEntry and Entry types needing
to align with libdom's FileSystemDirectoryEntry & FileSystemEntry.
This introduces 3 slight irregularities in order to be compatible with libdom
1. getParent successCallback needs to be optional
2. getParent errorCallback needs to be typed as a DOMException, despite it being a FileError in reality
3. getFile and getDirectory success types need to be relaxed to the base Entry
### Testing
I've created a test repository with several typescript versions, and ensured each of these compiles without issue: https://github.com/peitschie/cordova-plugin-file-type-tests
### Checklist
- [x] I've run the tests to see all new and existing tests pass
- [ ] I added automated test coverage as appropriate for this change
- [ ] Commit is prefixed with `(platform)` if this change only applies to one platform (e.g. `(android)`)
- [ ] If this Pull Request resolves an issue, I linked to the issue in the text above (and used the correct [keyword to close issues using keywords](https://help.github.com/articles/closing-issues-using-keywords/))
- [ ] I've updated the documentation if necessary
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-file] peitschie commented on pull request #536: fix(types): Adjust types to fix incompatibilities with TypeScript 4.4+
Posted by "peitschie (via GitHub)" <gi...@apache.org>.
peitschie commented on PR #536:
URL: https://github.com/apache/cordova-plugin-file/pull/536#issuecomment-1419917025
> From my own understanding, these changes do not appear to be correct, but can you explain to me why they would be considered correct?
Your understanding here is correct, in that in order to remain compatible with the libdom interface, our only option is to make our own types inaccurate 😞
I haven't really found any alternative solution, as we sit on top of the global interface which libdom expects to control.
The only way that I can think of to keep fully accurate types would be to expose this under an alias instead so we don't clash with the types.
It's an unfortunate situation!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-file] cadesalaberry commented on pull request #536: fix(types): Adjust types to fix incompatibilities with TypeScript 4.4+
Posted by GitBox <gi...@apache.org>.
cadesalaberry commented on PR #536:
URL: https://github.com/apache/cordova-plugin-file/pull/536#issuecomment-1337037317
How can we help on this PR to get it merged?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-file] cadesalaberry commented on pull request #536: fix(types): Adjust types to fix incompatibilities with TypeScript 4.4+
Posted by "cadesalaberry (via GitHub)" <gi...@apache.org>.
cadesalaberry commented on PR #536:
URL: https://github.com/apache/cordova-plugin-file/pull/536#issuecomment-1418892097
@erisu can we help?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-file] peitschie closed pull request #536: fix(types): Adjust types to fix incompatibilities with TypeScript 4.4+
Posted by "peitschie (via GitHub)" <gi...@apache.org>.
peitschie closed pull request #536: fix(types): Adjust types to fix incompatibilities with TypeScript 4.4+
URL: https://github.com/apache/cordova-plugin-file/pull/536
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org
[GitHub] [cordova-plugin-file] erisu commented on pull request #536: fix(types): Adjust types to fix incompatibilities with TypeScript 4.4+
Posted by "erisu (via GitHub)" <gi...@apache.org>.
erisu commented on PR #536:
URL: https://github.com/apache/cordova-plugin-file/pull/536#issuecomment-1418922705
I dont use TypeScript much to be able to confirm the changes but see if someone can take a look and review.
From my own understanding, these changes do not appear to be correct, but can you explain to me why they would be considered correct?
For example.
Before PR:
```ts
getParent(successCallback: (entry: Entry) => void,
errorCallback?: (error: FileError) => void): void;
```
After PR:
```ts
getParent(successCallback?: (entry: Entry) => void,
errorCallback?: (error: DOMException) => void): void;
```
This is telling users that the error's callback error property is `DOMException` but when looking at the actual code, this is not true.
The [`getParent` method in the `Entry.js`](https://github.com/apache/cordova-plugin-file/blob/master/www/Entry.js#L255) file, is clearly passing the [`FileError`](https://github.com/apache/cordova-plugin-file/blob/master/www/FileError.js)
`FileError.js` appears to have added extra APIs for the File API specification.
Same with the `getFile` and `getDirectory` method. It changes the success callback to `Entry` but in code respecticly it returns `FileEntry` and `DirectoryEntry`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscribe@cordova.apache.org
For additional commands, e-mail: issues-help@cordova.apache.org