Skip to content

[BUGFIX LTS] Warn about invalid properties#15213

Merged
rwjblue merged 2 commits intoemberjs:lts-2-12from
Serabe:fix/add-warning-for-invalid-brace-expansions
May 12, 2017
Merged

[BUGFIX LTS] Warn about invalid properties#15213
rwjblue merged 2 commits intoemberjs:lts-2-12from
Serabe:fix/add-warning-for-invalid-brace-expansions

Conversation

@Serabe
Copy link
Member

@Serabe Serabe commented May 7, 2017

There was a bug in the brace expansion implementation before 2.13 that
admitted expressions like a.{b,c and a.b,c in property expansion
without failing.

On February 2017, #13231 was merged improving the implementation of
expandProperties and fixing the previous issue.

Sadly, people upgrading from 2.12 are having problems with this. We have
two options here:

  • Backporting Correctly handle commas in CP property keys #13231 fixes it and can be done, as of today, without any
    problem just by cherry-picking its two commits. Sadly, this would
    break people apps in a patch version, and this bug does not seem like
    a critical bugfix.

  • Adding a warning if a user is using invalid properties. I went with
    this option since it looks like the less intrusive one while letting
    the users know that their apps will break in the next update.

@Serabe
Copy link
Member Author

Serabe commented May 7, 2017

I used warn because I did not feel like we were deprecating a feature, but feedback is welcome as always!

@rwjblue
Copy link
Member

rwjblue commented May 8, 2017

Seems good to me. Choosing between warn and deprecate here is somewhat hard, but I agree that this isn't really a "deprecation" per-se.

Serabe added 2 commits May 9, 2017 00:08
There was a bug in the brace expansion implementation before 2.13 that
admitted expressions like `a.{b,c` and `a.b,c` in property expansion
without failing.

On February 2017, emberjs#13231 was merged improving the implementation of
`expandProperties` and fixing the previous issue.

Sadly, people upgrading from 2.12 are having problems with this. We have
two options here:

- Backporting emberjs#13231 fixes it and can be done, as of today, without any
  problem just by cherry-picking its two commits. Sadly, this would
  break people apps in a patch version, and this bug does not seem like
  a critical bugfix.

- Adding a warning if a user is using invalid properties. I went with
  this option since it looks like the less intrusive one while letting
  the users know that their apps will break in the next update.
@Serabe
Copy link
Member Author

Serabe commented May 8, 2017

Updated messages to depend on the reason it stopped working. Properties like model.{baz will not longer work but expressions like a,b will continue to work though they will not expand.

Thank you!

@Serabe Serabe force-pushed the fix/add-warning-for-invalid-brace-expansions branch from 9b76588 to d9217ed Compare May 8, 2017 22:34
@rwjblue rwjblue merged commit 972ee0f into emberjs:lts-2-12 May 12, 2017
@rwjblue
Copy link
Member

rwjblue commented May 12, 2017

Thank you @Serabe!

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

Successfully merging this pull request may close these issues.

2 participants