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

Compatibility with Ember 4? #2

Closed
NullVoxPopuli opened this issue Dec 23, 2021 · 17 comments
Closed

Compatibility with Ember 4? #2

NullVoxPopuli opened this issue Dec 23, 2021 · 17 comments

Comments

@NullVoxPopuli
Copy link

I got some errors with: https://github.com/NullVoxPopuli/ember-three-boxes-demo

specifically on : https://github.com/NullVoxPopuli/ember-three-boxes-demo/blob/master/app/components/demo/without-templates/scene/box.js#L27

Wanted to double check first tho, did I hook this up right? anything obvious missing?

import Component from '@glimmer/component';
import { effect } from 'ember-tracked-effects-placeholder';
import { registerDestructor } from '@ember/destroyable';

import THREE from 'three';

let geometry = new THREE.BoxGeometry(2, 2, 2);
let material = new THREE.MeshNormalMaterial();

export default class Box extends Component {
  constructor(owner, args) {
    super(owner, args);

    this.mesh = new THREE.Mesh(geometry, material);

    let r = args.rotation.r;
    this.mesh.rotation.set(r.x, r.y, r.z);
    this.mesh.position.set(0, 0, 0);

    args.scene.add(this.mesh);

    registerDestructor(this, () => {
      args.scene.remove(this.mesh);
    });
  }

  @effect box = () => {
    let { rotation } = this.args;
    let r = rotation.r;

    if (!this.mesh) return;
    this.mesh.rotation.set(r.x, r.y, r.z);
  };
}
@lifeart
Copy link

lifeart commented Jan 18, 2022

Hi @NullVoxPopuli! Could you describe kind of error? Is effect not working correctly? (not updating, updating too often)

@lifeart
Copy link

lifeart commented Jan 18, 2022

I see
image

image

.backburner now ._backburner

https://github.com/emberjs/ember.js/search?q=_backburner

@BryanCrotaz
Copy link
Owner

That's the private api bit. We'll need to find a different way in Ember 4.

@lifeart
Copy link

lifeart commented Jan 18, 2022

@BryanCrotaz we don't really need to use backburner here...

@BryanCrotaz
Copy link
Owner

The reason it's there is that there's always a backburner loop when there's a data change, which saves polling continuously. What we really want is an event when a tracking tag is invalidated.

@lifeart
Copy link

lifeart commented Jan 20, 2022

@BryanCrotaz we already could have it, adding setter to version property, see my original gist

@Atrue
Copy link

Atrue commented Feb 21, 2022

@BryanCrotaz The reason that people are making attempts to create effects for Ember is simple: Ember doesn't have it yet.
I believe the effects will be a native Ember feature, but until that, we'll need some library that covers as many as possible cases for now. And I think this library may cover them. I checked a lot of prototypes of effects and this approach is the only one I liked.
So I think all this is a reason to use a private API. Anyway it already uses a backburner instance (which is private), so it can use just backburner or _backburner property depending on the ember version or just have a simple check:

import runloop from "@ember/runloop";
const backburner = runloop.backburner || runloop._backburner;

@BryanCrotaz
Copy link
Owner

I'll make that change.

I've added ember-try so that we can test with all the Ember versions. Can anyone help diagnose the build errors? https://github.com/BryanCrotaz/ember-tracked-effects-placeholder/runs/5273892081?check_suite_focus=true

@BryanCrotaz
Copy link
Owner

BryanCrotaz commented Feb 21, 2022

node_modules/@types/ember-resolver/node_modules/@types/ember__object/index.d.ts:164:9 - 
error TS2717: Subsequent property declarations must have the same type.  
Property 'class' must be of type 'typeof EmberObject', but here has type 'typeof EmberObject'.

Huh??

@NullVoxPopuli
Copy link
Author

maybe try re-rolling the lockfile? I find that the easiest way to deal with issues with type deps

@BryanCrotaz
Copy link
Owner

Didn't work

@lifeart
Copy link

lifeart commented Feb 22, 2022

@BryanCrotaz it's related to types conflict

image

resolutions should be set to have only one version

@BryanCrotaz
Copy link
Owner

import runloop from "@ember/runloop";
const backburner = runloop.backburner || runloop._backburner;

Error:
@ember/runloop does not have a default export

@Atrue
Copy link

Atrue commented Feb 24, 2022

Yes... Ember < 4 doesn't work with wildcard imports ember-cli/babel-plugin-ember-modules-api-polyfill#16

But it turned out that Ember transform _backburner import to the correct instance. So import { _backburner } from "@ember/runloop"; with ts-ignore is working for Ember 3.16, 3.28 and 4.1 versions (I checked only 3 these versions)

As for github actions, if you never run it for this project I suggest creating an empty addon and making it work with empty-try, so later you can append all changes gradually to see what is the real reason of the error

@BryanCrotaz
Copy link
Owner

Nearly there - but might need some help.

In 'embroider-safe', it throws Module not found: Error: Can't resolve '@glimmer/tracking'

And in Ember 3.16 the decorator doesn't work - is this something you'd expect not to work in 3.16?

@Atrue
Copy link

Atrue commented Feb 24, 2022

Did you receive this in the final 3.16 application?
I see the tracking package is working fine, but what I see is the error about @ember/destroyable. This package was introduced only in 3.22 https://blog.emberjs.com/ember-3-22-released
I think you'll need to make the ember-try working first

@BryanCrotaz
Copy link
Owner

fixed in v 1.2.0

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

4 participants