Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Angular issues with 1.10 rc versions #1543

Closed
smnbbrv opened this issue Jun 19, 2019 · 9 comments
Closed

Angular issues with 1.10 rc versions #1543

smnbbrv opened this issue Jun 19, 2019 · 9 comments

Comments

@smnbbrv
Copy link
Member

smnbbrv commented Jun 19, 2019

Problem:

Hi @owen-m1 and Co,

The angular users report, that the library does not work after upgrade to rc versions.

Are there known breaking changes? What could be the issue that the code that worked previously (1.7 -> 1.9) stopped working now

See SortableJS/ngx-sortablejs#163

JSBin/JSFiddle demonstrating the problem:

Please see the attached issue


Before you create an issue, check it:

  1. Try master-branch, perhaps the problem has been solved;
  2. Use the search, maybe we already have an answer;
  3. If not found, create an example on jsbin.com (draft) and describe the problem.

Bindings:

@owen-m1
Copy link
Member

owen-m1 commented Jun 19, 2019

I took a look at it, and I am not sure. There aren't any error messages, so there must be an issue with ngx-sortablejs interacting with Sortable somehow. There aren't any breaking changes that I know of between 1.9.0 and 1.10.0-rc2.

I think you are in a better position for figuring this out than me. The fact that it works on StackBlitz and not locally makes me think it has something to do with the way angular is building it. But again, I have no idea :/

The only thing I can think of is that this is now the entry point of the ESM that is imported when import Sortable from 'sortablejs' is called: https://github.com/SortableJS/Sortable/blob/master/entry/entry-defaults.js
But it still has Sortable as the default export, so I don't know why that would affect it.

@smnbbrv
Copy link
Member Author

smnbbrv commented Jun 19, 2019

Thanks, the latter could be very useful! I’ll check the ways how it can be imported differently

@smnbbrv
Copy link
Member Author

smnbbrv commented Jun 21, 2019

Here is the output of require('sortablejs')

1.9.0 (and former):

image

1.10.rc-2

image

That's a breaking change still... I have nothing against, just want to know whether it will be kept like that.

Doesn't this version deserve to be 2.0.0?

@owen-m1
Copy link
Member

owen-m1 commented Jun 21, 2019

@smnbbrv I never thought about anyone using the require syntax.
It is backwards compatible for import Sortable from 'sortablejs';, because the default export is the same, but it seems the require function does not respect default exports.

Is this what is causing ngx-sortablejs to break? If so, where is this require method being called in the code/build?

@smnbbrv
Copy link
Member Author

smnbbrv commented Jun 21, 2019

Yeppers, here is a permalink

https://github.com/SortableJS/ngx-sortablejs/blob/337eb0d3ab81cd144d8e35ca815c1e2b0886aea0/projects/ngx-sortablejs/src/lib/sortablejs.directive.ts#L8

As I can remember formerly I was forced to use require syntax because of the broken typescript typings. Now I can rework it in a way it uses default export, so it is fine for me.

However this is going to be closely connected to the #1547 . The typings are messy now.

I would close this issue then and raise a newer (probably a major version) on ngx-sortablejs in the near future.

@smnbbrv smnbbrv closed this as completed Jun 21, 2019
@waynevanson
Copy link
Contributor

waynevanson commented Oct 11, 2019

@owen-m1 @smnbbrv Is this actuall allowed in the sortablejs package?

import { Sortable } from 'sortablejs';

I'm in the middle of a PR and can add this in if it works, I just need confirmation.

@owen-m1
Copy link
Member

owen-m1 commented Oct 11, 2019

@waynevanson Yes you are able to import Sortable that way

@waynevanson
Copy link
Contributor

@owen-m1 Can you link to source code for this? I wanna make sure I get the correct typings #1547

@owen-m1
Copy link
Member

owen-m1 commented Apr 14, 2020

@waynevanson

Sortable,

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

No branches or pull requests

3 participants