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

Resizable options no longer working with ember hash helper Object.create(null) #1195

Closed
btecu opened this issue Mar 8, 2020 · 8 comments · Fixed by #1244
Closed

Resizable options no longer working with ember hash helper Object.create(null) #1195

btecu opened this issue Mar 8, 2020 · 8 comments · Fixed by #1244

Comments

@btecu
Copy link
Contributor

btecu commented Mar 8, 2020

After upgrading from 0.6.0 to 1.x, looks like setting the resizable options is no longer an option.
Using yahoo/ember-gridstack, although I suspect the issue is not with the addon but GridStack itself.

Here is how being invoked:

<GridStack @options={{hash resizable=(hash handles="e")}}>
  // items here
</GridStack>

Here is GridStack being initialized in the addon:
image

The error is not very useful:
image

If I remove resizable it works fine, except I can't change the handles value.

@btecu
Copy link
Contributor Author

btecu commented Mar 8, 2020

Disregard, it appears to be an issue with the object being created by the hash helper.

@btecu btecu closed this as completed Mar 8, 2020
@btecu btecu reopened this Mar 8, 2020
@btecu
Copy link
Contributor Author

btecu commented Mar 8, 2020

Appears to work when constructing the options in JS and passing them:
image

However the hash just creates an object as you can see in the debug. Maybe still worth looking into it?

@adumesny
Copy link
Member

adumesny commented Mar 8, 2020

hey can you post a jsfiddle showing the issue (with hash or whatever triggers the issue)? as I don't have access to ember to debug this. weird that it would work in 0.6.x but not 1.x on my side...

cool I see you are using the new oneColumnModeDomSort option. Curious how you are using the improved

@btecu
Copy link
Contributor Author

btecu commented Mar 8, 2020

@adumesny not quite a JSFiddle, but close:
https://ember-twiddle.com/b319aa6e99171e6038ce17bd02d0fdc9?openFiles=templates.application%5C.hbs%2C

Notice the error in the console and how removing the @options/resizable fixes the issue.

@btecu
Copy link
Contributor Author

btecu commented Mar 9, 2020

@adumesny It appears that the issue is that the passed in object doesn't have hasOwnProperty.

@adumesny adumesny changed the title Resizable options no longer working Resizable options no longer working with hash helper Mar 21, 2020
@adumesny
Copy link
Member

hey let me know if you find a fix or idea on how to fix it.
I'm focusing on 2.x now but could do a 1.x release if you have an idea...

@btecu
Copy link
Contributor Author

btecu commented Mar 23, 2020

The issue is here:

if (source.hasOwnProperty(prop) && (!target.hasOwnProperty(prop) || target[prop] === undefined)) {

The Ember helper creates an object using Object.create(null) therefore not having a prototype. However, the code here relies on the prototype to call hasOwnProperty.

There are a few solutions here:

  • Check if the value directly: source.hasOwnProperty(prop) -> source[prop] !== undefined
  • Use the Object prototype: Object.prototype.hasOwnProperty.call(source, prop)
  • Do nothing, seems like an edge case

@adumesny
Copy link
Member

adumesny commented Mar 23, 2020

  1. field missing vs being undefined are different though I don't think we depend on undefined to set/clear a field.
  2. that might be safest fix
  3. don't you need it fixed ? seems weird your object doesn't have a prototype. Should be {} based no ?

@adumesny adumesny changed the title Resizable options no longer working with hash helper Resizable options no longer working with ember hash helper Object.create(null) Mar 23, 2020
adumesny added a commit to adumesny/gridstack.js that referenced this issue Mar 29, 2020
adumesny added a commit that referenced this issue Mar 29, 2020
adumesny added a commit to adumesny/gridstack.js that referenced this issue May 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants