search

alexlafroscia / ember-steps

Declaratively create wizards, tabbed UIs, and more
https://alexlafroscia.github.io/ember-steps
MIT License
90 stars 23 forks source link

Error with Ember.js 2.15.0 #78

Closed YoranBrondsema closed 7 years ago

YoranBrondsema commented 7 years ago

I'm getting the following error with Ember.js 2.15.0.

TypeError: Cannot read property 'concat' of undefined
    at WrappedBuilder.compile (runtime.js:3096)
    at ComponentLayoutBuilder.compile (runtime.js:2989)
    at compileLayout (runtime.js:2970)
    at Cache._emberMetal.Cache.owner [as func] (environment.js:84)
    at Cache.get (ember-metal.js:2436)
    at Environment.getCompiledBlock (environment.js:159)
    at CurlyComponentManager.layoutFor (curly.js:225)
    at runtime.js:1716
    at AppendOpcodes.evaluate (runtime.js:70)
    at VM.next (runtime.js:7244)

It seems to be an error thrown in @glimmer/runtime v0.25.3: https://github.com/glimmerjs/glimmer-vm/blob/v0.25.3/packages/%40glimmer/runtime/lib/compiler.ts#L183.

I tried to replicate it in this addon by upgrading the Ember version but to no avail. So I'm guessing it's something specific to my application. Just putting it out here in case someone sees the same error.

alexlafroscia commented 7 years ago

Interesting...

This add-on does do some funky build-time stuff to handle weird cases like steps that weren't given a name. As someone that uses the add-on -- would it be annoying if I removed that feature? Always having a name set removes some of the weird stuff I currently do, and less weird stuff means less areas that are likely to break as glimmer/ember update

YoranBrondsema commented 7 years ago

You mean the code in https://github.com/alexlafroscia/ember-steps/blob/master/lib/htmlbars-plugin/index.js? I never realized that that code was there in the addon haha.

One thing tells me it's not there as my app fails when trying to render a step-manager, not at build time. However, I need to test this assumption and do some more debugging into what part of the addon is causing the error.

That aside, I agree with you and I don't think the build time checks are necessary. They would only come up when you start the Ember.js server. So if you make some changes during development and omit a parameter, the build-time checks won't catch it anyway. However, you have a good set of runtime checks in the init hooks of the components which catch those and imo they are sufficient. Any test in your app that renders an erroneous step-manager somewhere would catch, so unless you have like 0.4% test coverage the chances are high that a test would catch it. So yes, I'm all for reducing complexity and removing the build-time checks!

alexlafroscia commented 7 years ago

Alright, I'll look into removing the build-time stuff that I'm doing right now to limit the complexity a bit. I'm not sure if I can remove all of it, but it's worth re-visiting. It's really hard to write tests that do custom HTMLBars stuff.

alexlafroscia commented 7 years ago

More relevant: if you can provide a Twiddle or example repo with a reproduction, I'm happy to debug. I just can't do much without one of those two things (preferably a full repo)

alexlafroscia commented 7 years ago

It shouldn't be an error with Ember 2.15 specifically, since the tests are run against the Canary builds (and just passed today)

YoranBrondsema commented 7 years ago

You're probably right about that. I'll debug some more on my side to try to pinpoint the circumstances that make it fail. To be continued :-)

YoranBrondsema commented 7 years ago

I'm closing cause now I can't replicate it anymore.

alexlafroscia commented 7 years ago

Thanks @YoranBrondsema, I appreciate the diligence in closing this out. Helps me stay on top of things!