ember-cli-head icon indicating copy to clipboard operation
ember-cli-head copied to clipboard

Add a warning to readme about using `model` in Octane

Open ijlee2 opened this issue 4 years ago • 5 comments

Hello. A couple of weeks ago, my team introduced a bug when we Octanified our templates by changing all instances of this.model to @model. We didn't realize until today that we should have left alone this.model in app/templates/head.hbs, since model refers to the head-data service.

I was wondering if you could (1) add a warning to the readme file under https://github.com/ronco/ember-cli-head#service, and (2) update the example code to Octane,

<meta property="og:title" content={{this.model.title}} />

so that other developers and teams may avoid our mistake when using Octane? I can also create a PR if you think making these changes would be a good idea.

ijlee2 avatar Jan 20 '20 21:01 ijlee2

I think we could even go a step further and not name it model to avoid confusion, but at the very least, I think we should do as @ijlee2 suggests.

RobbieTheWagner avatar Jun 18 '20 01:06 RobbieTheWagner

Sounds good. I can make a PR this weekend to add the warning and update the example code.

I think renaming model to a more descriptive name would be good. However, since this will cause a breaking change, it may be out of the extent of help that I can offer.

ijlee2 avatar Jun 18 '20 18:06 ijlee2

@ijlee2 yeah, I think no need to introduce a breaking change right now, but we do need to guard people against making this mistake.

RobbieTheWagner avatar Jun 18 '20 19:06 RobbieTheWagner

Ya, we can expose a new field though. So you could author this.headData.someProperty, instead of this.model. Using model here really just isn't very useful.

rwjblue avatar Jun 18 '20 22:06 rwjblue

I opened the PR at https://github.com/ronco/ember-cli-head/pull/81, just for updating the readme. I suggest that we make a new issue to address the ambiguity in model.

ijlee2 avatar Jun 20 '20 16:06 ijlee2