Skip to content

Conversation

@kaffarell
Copy link
Collaborator

Will fix #560

@coveralls
Copy link

coveralls commented Oct 14, 2020

Coverage Status

Coverage increased (+1.3%) to 78.082% when pulling e9373d2 on update-typescript-plugin into 97e33f2 on master.

Bumps [tar](https://github.com/npm/node-tar) from 4.4.1 to 4.4.13. **This update includes security fixes.**
- [Release notes](https://github.com/npm/node-tar/releases)
- [Changelog](https://github.com/npm/node-tar/blob/master/CHANGELOG.md)
- [Commits](isaacs/node-tar@v4.4.1...v4.4.13)

Signed-off-by: dependabot-preview[bot] <[email protected]>
@atodorov
Copy link
Collaborator

atodorov commented Oct 16, 2020

Will fix #560

Can you add this to commit log and also rebase b/c the latest master branch updated a bunch of other dependencies.

Otherwise LGTM so not sure why the subject says "WIP".

@kaffarell
Copy link
Collaborator Author

kaffarell commented Oct 16, 2020

Ok, have done the changes. I included WIP in the title because there are a lot of other error down the line. I removed the IE fix but I cannot check if it is used because IE11 is still not working ( #445 ). The problems are typescript errors, every usage of options is using the configuration type but it also has other types.

* Public sortable object
* @param {Array|NodeList} sortableElements
* @param {object|string} options|method
*/
export default function sortable (sortableElements, options: configuration|object|string|undefined): sortable {
// get method string to see if a method is called
const method = String(options)
options = options || {}

So I could add a <configuration> before every usage of options but I don't think this is the best solution. I haven't found out when the object string or undefined type on options is used, so we could maybe remove them and write:
options: configuration

@kaffarell
Copy link
Collaborator Author

All the errors look like this:

semantic error TS2339: Property 'items' does not exist on type 'string | void | configuration | HTMLElement'.
  Property 'items' does not exist on type 'string'.

@lukasoppermann
Copy link
Owner

Hey @kaffarell please verify what I am saying, but I think you could cast options to here:

options = Object.assign({}, defaultConfiguration, store(sortableElement).config, options)
because at this point it will always be a configuration, correct?

Otherwise we would probably need to add a function that deals with the options parameter and returns a configuration, but try the above mentioned first please.

@lukasoppermann
Copy link
Owner

@kaffarell any news so this can be made mergable?

@lukasoppermann lukasoppermann self-assigned this Aug 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The dependecy 'rollup-plugin-typescript' is deprecated

5 participants