Converts ES6 native classes to ember objects

[fran-worley]

This is a breaking change as it converts the new Class() syntax to Class.create().
In addition, the only way I could get these to work was to convert the positional arguments to named args in an options hashes:

new Table(columns, rows) => Table.create(columns: columns, rows: rows)
new Row(content, options) => Row.create({ content: content })
new Column(opts) => Column.create(opts)

We’ve had to replace the native constructor methods with emberObject init methods as per the official docs which should also fix #699. As a result we’ve removed the Ember 2.12 ‘hacks’ which shouldn’t be an issue as we’ve bumped ember version minimum version to 2.18.

Clearly there are issues with ember 2.18 that need resolving but @alexander-alvarez what are your thoughts so far?

[fran-worley]

Would it help to release a codemod for this too? I'm also unsure of what to do about the docs as ELT's aren't versioned. Could we enable something like http://offirgolan.github.io/ember-cp-validations/ so we can maintain 1.x and 2.x versions?

[richard-viney]

Looks like as of Ember 3.11 the changes in this branch are now mandatory, i.e. master of ember-light-table doesn't function with [email protected]. It gives the error:

EmberObject based class, (unknown), was not instantiated correctly. You may have either used `new` instead of `.create()`, or not passed arguments to your call to super in the constructor: `super(...arguments)`. If you are trying to use `new`, consider using native classes without extending from EmberObject.

The good news is that this branch is working well in my testing so far 👍.

[fran-worley]

@richard-viney any chance you could help me fix this back to 2.18 then I can merge!

[fran-worley]

@alexander-alvarez any thoughts on the best way to handle the docs? I'm keen to get this merged ASAP as without it, ELT doesn't work with Ember 3.11+.

Should we just put a note in the readme on the 2.x branch for now - we'll need to do something more permanent before 2.x release though

[alexander-alvarez]

Should we just put a note in the readme on the 2.x branch for now - we'll need to do something more permanent before 2.x release though

👍
sorry for the delay american holidays

[richard-viney]

@fran-worley I would have thought that the ember-native-class-polyfill package was needed to make this work? The tests are reportedly passing on 3.4 even without it though, which is curious.

The native class polyfill only supports Ember 3.4/3.5, as native class support was added in 3.6. It doesn't claim to work on 2.18 though, not sure if there were technical limitations preventing this from being possible.

It would be nice to support 2.18 if we can, though it does appear to be out of extended support so maybe it's not necessary: https://emberjs.com/releases/.

[fran-worley]

@richard-viney I didn't think we'd need it as if you look at the PR we're not using 'native classes' as such. I've converted it back to more traditional Ember Objects largely due to the limited support for the polyfill.

[richard-viney]

Ok, in that case is there a reason to keep writing

export default class Table extends EmberObject.extend({
    // ...
}) {};

instead of

const Table = EmberObject.extend({
    // ...
});

export default Table;

Is writing class here actually meaningful? The latter seems like it may be more backwards compatible if that's what we're going for. No idea if this is related to the test failures though!

[fran-worley]

@richard-viney I honestly have no idea - I did this so long ago now and it was a bit of a stretch as javascript is not my first language and the intricacies of classes vs ember objects hasn't ever really sunk into my brain!

I'll take another look this week and see if I can work it out.

[lougreenwood]

To throw a spanner in the works...

I discussed this issue with @pzuraq this afternoon on Discord after I came across it in my app when upgrading to Ember 3.11. It seems that the recommended path (in the long term) for fixing this issue is to migrate away from EmberObject to native classes for these kind of utility classes.

By using native classes, it's also not a breaking API change since new can continue to be used (AFAIK, this PR mandates .create()).

However, I suppose it would be a breaking change since it drops support for at least Ember 2.18 (or whenever the native class polyfill goes back to).

[fran-worley]

@lougreenwood we talked about going down this route initially but with the native polyfill support being limited (and I've seen other addons encounter issues with it) and with the intention of this addon to move to conventional .create syntax (see #444 ) we decided that the easiest option was to 'break' the api to use convention ember objects.

The road to pure native classes in ember is likely to be a while longer yet and depending on how it is implemented may end up 'breaking' our API anyway.

[lvegerano]

I now this is a WIP. Ember 3.11 totally breaks the addon. I'm trying to see if I can use this branch but Im running into an issue when occlusion is turned off. I basically get a compile error coming from lt-body.hbs. We're using angle-bracket invocation as well.

[richard-viney]

@lvegerano Are you configuring the ember-composable-helpers addon in such a way that excludes helpers needed by ember-light-table? E.g. by using its only: [...] config option? I believe it needs the compute helper at a minimum, is that the helper that's causing the exception?

[fran-worley]

@lougreenwood in addition to @richard-viney suggestion angle bracket syntax is as yet untested and I know people have reported issues with it. Our aim is to get the PR merged ASAP and then work on Angle Bracket Syntax support.

[lvegerano]

@richard-viney not really, everything is default

[basz]

I'm trying to use this branch in an ember 3.11.0 app however the same error occurs... Do understand it correctly it should fix the 'not instantiated correctly' problem?

package.json

"ember-light-table": "offirgolan/ember-light-table#refs/pull/701/head",

[richard-viney]

@basz This is what I'm using in package.json:

"ember-light-table": "github:offirgolan/ember-light-table#fw/native-classes-ember-obj",

[fran-worley]

Sorry for the silence on my part - I'm taking an executive decision to press ahead with this fix and therefore withdrawing support for Ember 2.18 (at least for the time being). With ember 3.12 having just been released as the new LTS I think it's more important we support the future than the past.

I believe the compile error issue you're experiencing is likely due to #693 where I moved ember-composable-helpers to a dev dependency so I'll restore it and see where we get to.

I'm hoping to get this merged and a new beta release out this weekend and from there we can move onto to looking at Angle Bracket syntax etc.

@alexander-alvarez what is the situation with the documentation site as this change breaks the API. For now the best I can do is update the readme and put a warning notice on the master readme to advice people of the change in the v2x branch but we should update the main docs sooner rather than later

[alexander-alvarez]

I'm hoping to get this merged and a new beta release out this weekend and from there we can move onto to looking at Angle Bracket syntax etc.
👍

@fran-worley once docs are updated pushing to gh-pages branch will release them. agree with getting the docs updated sooner rather than later, but need to make sure that it's clear what version is supported by the docs syntax

[jking6884]

@basz This is what I'm using in package.json:

"ember-light-table": "github:offirgolan/ember-light-table#fw/native-classes-ember-obj",

I added this to my package.json and ran npm install. But got the same error. I deleted node_modules and reinstalled. Still the same issue though.

Is there anything I am missing?

[richard-viney]

@jking6884 Do you any of the admins you're using depend on ember-light-table? If so. they may need to be updated. Trace the call stack to find the call to new Table() and then look into why the code is still doing that (it's no longer supported).

[fran-worley]

@alexander-alvarez is it possible to have 2 doc sites? one for the 1.x branch and another for the 2.x branch? I can see the 2.x branch deviating even further as we look to support angle bracket syntax etc.

[jking6884]

@fran-worley @richard-viney

Isn't this site intended to be 'documentation' of how to use the addon: https://offirgolan.github.io/ember-light-table/

  init() {
    this._super(...arguments);

    let table = new Table(this.get('columns'), this.get('model'), { enableSync: this.get('enableSync') });
    let sortColumn = table.get('allColumns').findBy('valuePath', this.get('sort'));

    // Setup initial sort column
    if (sortColumn) {
      sortColumn.set('sorted', true);
    }

    this.set('table', table);
  },

  fetchRecords: task(function*() {```

[fran-worley]

@jking6884 that covers v1 of the admin. This PR changes the API to be compliant with ember 3.11. See #701 (comment) .

Docs will be updated in due course but as we're all 'volunteers' please be patient!! 😅

[ghost]

Hi! Firstly, thanks for working on these issues. We really appreciate it. I am a little confused if v2.0.0-beta.4 contains the fix for this ticket. I have updated my package.json to include this version, and although I am not getting any errors anymore, my tables will not load. I'm generating the table using the following code:

Table.create(this.columns, users, { enableSync: true })

but the created table is empty, and only my isEmpty messaging appears. Can you confirm this is the release I should be using if I'm on Ember 3.11?

[fran-worley]

@Kira-Pilot91 hey - it is but per the first comment #701 (comment) you need to change how you initialise your tables.

We still need to update the docs.

[ghost]

My bad, I just noticed I wasn't passing in the arguments in the correct format. Sorry about that and thanks for the quick reply.

[Finn10111]

Will this also be merged into 1.x of ember-light table? Sorry for the late question but I just ran across this issue after upgrading to Ember.js 3.11.

[petervtzand]

I upgraded Ember.js from 3.10 to 3.11, and running ember-light-table 2.0.0-beta.4 and using native classes. My component looks like this:

// app/components/tables/-overview-table.js
import classic from 'ember-classic-decorator';
import Component from '@ember/component';
import Table from 'ember-light-table';

@classic
export default class OverviewTable extends Component {
  init() {
    this._super(...arguments);
    let table = Table.create({ columns: this.get('columns'), rows: this.get('model') });
    this.set('table', table);
  }
}

But I still get the error:

Uncaught Error: Assertion Failed: You must call this._super(...arguments); when overriding init on a framework object. Please update <my-app-name@service:media::ember328> to call this._super(...arguments); from init.

Am I doing anything wrong, or is it related to this issue?
P.S. tried it without native classes, but still same issue.
P.S. changing this._super(...arguments); into super.init(...arguments); doesn't work either

[petervtzand]

I found the issue, it is an issue of ember-responsive that is already fixed in version 3.0.5.

Update
I now see that I was using ember-responsive 3.0.0 myself, and because ELT uses "ember-responsive": "^3.0.5", which is 'compatible' with 3.0.0, it didn't upgrade it.
Maybe ELT has to set "ember-responsive": ">=3.0.5"??

[Gorzas]

Is there a reason to use EmberObject instead of native classes? I thought that the latest version of Ember recommends using native classes, isn't it?

[fran-worley]

Time!! Our maintainers have very little of it!
We plan to start work on this soon and will release it as v3.x. See our development roadmap #739