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] <support@dependabot.com>
Bumps [rollup](https://github.com/rollup/rollup) from 2.29.0 to 2.30.0. - [Release notes](https://github.com/rollup/rollup/releases) - [Changelog](https://github.com/rollup/rollup/blob/master/CHANGELOG.md) - [Commits](rollup/rollup@v2.29.0...v2.30.0) Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
Bumps [rollup](https://github.com/rollup/rollup) from 2.30.0 to 2.31.0. - [Release notes](https://github.com/rollup/rollup/releases) - [Changelog](https://github.com/rollup/rollup/blob/master/CHANGELOG.md) - [Commits](rollup/rollup@v2.30.0...v2.31.0) Signed-off-by: dependabot-preview[bot] <support@dependabot.com>
@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

5 participants