Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Breaking "bugfixes" for an eventual 3.0 #170

Closed
2 tasks
nathanhammond opened this issue Oct 10, 2016 · 18 comments
Closed
2 tasks

Breaking "bugfixes" for an eventual 3.0 #170

nathanhammond opened this issue Oct 10, 2016 · 18 comments

Comments

@nathanhammond
Copy link
Member

nathanhammond commented Oct 10, 2016

These are things that are very much broken but we can't change for reasons of backwards compatibility. This list is for a very specific category of issue. These should tend toward mechanical changes.

For example, query params clobber path params inside of the params object passed to the model hook. These should be separated into different keyspaces. This is breaking because it will change how people need to access query params and it's even possible that somebody was relying upon the clobbering behavior. However, with moving toward isolated query param behavior this tremendously reduces the complexity of the problem, skirts around different behaviors with refreshModel and simply setting a controller property, and makes the entire process more easily understood.

As a second example, query strings are serialized and deserialized differently in different layers of Ember. Changing this will obviously be breaking but it must be done to solve the "refresh" problem.

When you comment on this thread please identify why your issue fits this category. The canonical list will be hoisted here. Comments will only be used to identify which things should be promoted to this top post.


Unconfirmed as to whether they should be promoted:

@Gaurav0
Copy link
Contributor

Gaurav0 commented Oct 14, 2016

Computed properties without get or set readonly by default?

@btecu
Copy link

btecu commented Oct 17, 2016

  • Make get and set optional?
  • Make jQuery optional
  • Routable components? :trollface:

Sounds more like a wishlist.

@ghost
Copy link

ghost commented Oct 17, 2016

@nathanhammond
Copy link
Member Author

Confirm, these are wishlist items which is why I've not promoted them into the top-post. Y'all please keep this to things that are definitely broken but we can't change without likely breaking peoples applications.

For example, the params thing from my first post: query params clobber path params inside of the params object passed to the model hook. Almost certainly somebody relies on this behavior and fixing it requires separating the namespaces. Since that will change semantics and people will need to adjust where they look for properties it is breaking. That params is lossy is a bug.

This list is for a very specific category of issue.

@nathanhammond
Copy link
Member Author

Updated the top post to describe what we're looking for in this thread. Please review.

@runspired
Copy link
Contributor

runspired commented Oct 19, 2016

component.schedule and route.schedule instead of run.schedule (although mostly this can be achieved without a breaking change, I believe we should deprecate global Ember.run)

@nathanhammond
Copy link
Member Author

The function signature for the existing "routing service" is distinct from the function signature of the same method name on the route and controller.

The existing routing service: transitionTo(routeName, models, queryParams, shouldReplace)

The underlying implementation on routes: transitionTo(routeName, ...models, options)

I feel like I've opened a bug on the second invocation in the past for unrelated reasons, but can't find it.

We should make these consistent.

@knownasilya
Copy link
Contributor

knownasilya commented Oct 27, 2016

Keep inline link-to consistent with block format.

{{link-to Name route}}

vs

{{#link-to route}}
  Name
{{/link-to}}

The order is backwards and gets me every time I use the inline form. I'd expect:

{{link-to route Name}}
{{!-- or for clarity --}}
{{link-to route label=Name}}

@Serabe
Copy link
Member

Serabe commented Oct 27, 2016

Classic actions as {{action "actionName"}}

@nathanhammond
Copy link
Member Author

@knownasilya I'm personally of the opinion that all of inline {{link-to}} was a mistake. Also any triple-curly {{{link-to}}}. Also {{#link-to}} in general. I'd instead rather deprecate it entirely, but these changes (as well as your proposals) are beyond the scope of this issue.

@Serabe Your proposal also doesn't meet the bar for this thread, it should almost be something that is an unintended side-effect of a feature's implementation.

Both of y'all's proposals have to do with changing documented public API and that would require a deprecation and process. For future travelers if something has ever appeared in the guides there is no way it will be accepted into the top-posted list as that does not count as a bugfix.

@cibernox
Copy link
Contributor

@nathanhammond I agree on deprecating inline {{link-to}}
@Gaurav0 Agree on making computed-properties without a get being readonly by default, although probably some changes already in the pipeline for computed-properties when decorators can be used will already achieve that.

Deprecate component.sendAction (perhaps as part of glimmer-components) so people uses closure actions instead.

@balinterdi
Copy link

As the implementer of the inline link-to I feel intrigued :) I understand why the param order should be brought in line (pun intended) with the block form, but do you think it should be scrapped? It does save some typing and thus potential messing up, doesn't it?

@nathanhammond
Copy link
Member Author

@balinterdi My complaints are mostly that the entirety of link-to should go away. Positional arguments that get turned into content are also really a mess... most of Ember doesn't leverage positional arguments so I want to make that consistent since it's a non-zero logical overhead.

@balinterdi
Copy link

@nathanhammond Ok, that makes sense. So what is it going to get replaced by?

@rlivsey
Copy link

rlivsey commented Nov 21, 2016

@balinterdi I think the plan is to decompose link-to into a number of helpers & the routing service along the lines of:

<a href={{url-for "foo"}} class={{active-class "foo"}}>
  Name
</a>

This has the added benefit that url-for & friends can be used outside of the context of rendering a link, eg for passing to components / translation libraries, for more easily setting the active class on container elements etc…

link-to could still exist and just use those smaller building blocks vs implementing everything itself in a black box.

@balinterdi
Copy link

@rlivsey Now this makes perfect sense, thank you for the explanation!

@knownasilya
Copy link
Contributor

knownasilya commented Jan 4, 2017

What about emberjs/ember.js#14783 I'd call that a bug, even though some wouldn't.

@sandstrom
Copy link
Contributor

@nathanhammond Can we close this issue? I guess it's not relevant anymore

@rwjblue rwjblue closed this as completed May 20, 2020
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

No branches or pull requests