emberjs / ember.js

Ember.js - A JavaScript framework for creating ambitious web applications
https://emberjs.com
MIT License
22.47k stars 4.21k forks source link

TypeError: with Native Class syntax on certain lifecycle hooks when calling `super` #17156

Closed alexdiliberto closed 6 years ago

alexdiliberto commented 6 years ago

Converting an app over to use native classes and noticed some component hooks throw errors when calling their superclass equivalent hook. didReceiveAttrs is an example of one hook which will error

@rwjblue helped answer my question today on Discord

rwjblue Today at 1:52 PM
its a bug
basically the parent class doesn't have a didReceiveAttrs hook implemented

import Component from '@ember/component';
export default class MyClass extends Component {
  didReceiveAttrs() {
    // This will throw a TypeError
    super.didReceiveAttrs(...arguments);

    // ... other stuff
  }
}
TypeError: (intermediate value).didReceiveAttrs is not a function
    at ModalGenericFooter.didReceiveAttrs (generic-footer.js:21)
    at ModalGenericFooter.trigger (core_view.js:62)
    at ModalGenericFooter.superWrapper [as trigger] (ember-utils.js:308)
    at CurlyComponentManager.create (ember-glimmer.js:3697)
    at Object.evaluate (runtime.js:1261)
    at AppendOpcodes.evaluate (runtime.js:46)
    at LowLevelVM.evaluateSyscall (runtime.js:2895)
    at LowLevelVM.evaluateInner (runtime.js:2867)
    at LowLevelVM.evaluateOuter (runtime.js:2859)
    at VM.next (runtime.js:4787)
    at VM.execute (runtime.js:4772)
    at TryOpcode.handleException (runtime.js:3752)
    at UpdatingVMFrame.handleException (runtime.js:3922)
    at UpdatingVM._throw [as throw] (runtime.js:3657)
    at Assert.evaluate (runtime.js:730)
    at UpdatingVM.execute (runtime.js:3644)
    at RenderResult.rerender (runtime.js:3949)
    at RootState._this30.render (ember-glimmer.js:4075)
    at TransactionRunner.runInTransaction (ember-metal.js:406)
    at InteractiveRenderer._renderRoots (ember-glimmer.js:4340)
    at InteractiveRenderer._renderRootsTransaction (ember-glimmer.js:4372)
    at InteractiveRenderer._revalidate (ember-glimmer.js:4412)
    at invokeWithOnError (backburner.js:283)
    at Queue.flush (backburner.js:186)
    at DeferredActionQueues.flush (backburner.js:353)
    at Backburner._end (backburner.js:808)
    at Backburner.end (backburner.js:558)
    at Backburner._run (backburner.js:847)
    at Backburner._join (backburner.js:829)
    at Backburner.join (backburner.js:612)
    at join (index.js:164)
    at has_element.js:18
    at flaggedInstrument (index.js:119)
    at Object.handleEvent (has_element.js:17)
    at Class.handleEvent (view_support.js:402)
    at HTMLButtonElement.<anonymous> (event_dispatcher.js:327)
    at HTMLBodyElement.dispatch (jquery.js:5183)
    at HTMLBodyElement.elemData.handle (jquery.js:4991)
pixelhandler commented 6 years ago

@alexdiliberto within the hooks, we should only call super.didReceiveAttrs if we know the parent calls has that same hook defined. Otherwise, calling super.didReceiveAttrs(...arguments); will error.

I think that may be part of the application refactoring needs, not necessarily an Ember bug.

That is how JavaScript works...

class Greet { constructor(msg) { this.msg = msg; } hello() { console.log(this.msg)} }
let g = new Greet('hi')
g.msg
"hi"
g.hello()
VM586:1 hi

class Yo extends Greet { hello() { console.log('Yo') }}
let y = new Yo('sup')
y.msg
"sup"
y.hello()
VM699:1 Yo

class Sup extends Yo { hello() { super.hello(); console.log('sup') }}
let s = new Sup()
s = new Sup('dog')
s.msg
"dog"
s.hello()
VM699:1 Yo
VM866:1 sup

class WhatUp extends Sup { yo() { super.yo(); console.log('what up ' + this.msg) }}
let w = new WhatUp('peeps')
w.msg
"peeps"
w.hello()
VM699:1 Yo
VM866:1 sup

w.yo()
VM1115:1 Uncaught TypeError: (intermediate value).yo is not a function
    at WhatUp.yo (<anonymous>:1:44)
    at <anonymous>:1:3

If @rwjblue thinks this is a bug in Ember then we'd need to change the Component hooks to define the methods as noop functions. I'm not sure why that is not the case now. See https://github.com/emberjs/ember.js/blob/v3.5.0/packages/ember-glimmer/lib/component.ts#L737-L745

So this would be a change request to the Component. Not a bug in Ember. Do the guides as of v3.5 suggest we use class syntax yet?

rwjblue commented 6 years ago

Ya, I think we should ensure that all of the component hooks are defined on the base class. It’s actually important unrelated to using native classes (it helps ensure consistent shapes and whatnot)...

pzuraq commented 6 years ago

@rwjblue if that's the case we should probably try to get this into 3.6, no? I'll submit a PR asap