search

ember-fastboot / ember-cli-head

Ember Cli Addon for Adding Content to HTML Head
MIT License
98 stars 34 forks source link

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

Open ijlee2 opened 4 years ago

ijlee2 commented 4 years ago

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.

RobbieTheWagner commented 4 years ago

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.

ijlee2 commented 4 years ago

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.

RobbieTheWagner commented 4 years ago

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

rwjblue commented 4 years ago

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.

ijlee2 commented 4 years ago

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.