These are chat archives for canjs/canjs

19th
Jan 2018
Runn Vermel
@runn-vermel
Jan 19 2018 00:36
@justinbmeyer @matthewp - that's weird - i walked away for two hours, and when i came back, the warnings were back - even with the "import CanLog from "can-log"; CanLog.logLevel = 99;"
Gregg Roemhildt
@roemhildtg
Jan 19 2018 13:44
Question on routing: If I have a route defined like route('my-page-path/{id}'); How do I use routeUrl(hashes) to get that path?
Matthew Phillips
@matthewp
Jan 19 2018 13:45
routeUrl(id=1)
Gregg Roemhildt
@roemhildtg
Jan 19 2018 13:46
So route will use the current path? What if my current route is /my-other-page and I want to link to /my-page-path/1
href="/my-page-path/{{id}}"?
Bitballs does it like this
route('tournaments/{tournamentId}');
route('games/{gameId}');
route('players/{playerId}');
route('{page}',{page: 'tournaments'});
I'm just not sure how to generate a route for tournaments/tournamentId
from like the game page for instance
Matthew Phillips
@matthewp
Jan 19 2018 14:06
routeUrl(page=tournaments tournamentId=2)
Gregg Roemhildt
@roemhildtg
Jan 19 2018 14:08
Okay
Matthew Phillips
@matthewp
Jan 19 2018 14:08
:)
Gregg Roemhildt
@roemhildtg
Jan 19 2018 14:08
Simpler than I thought
lol
Matthew Phillips
@matthewp
Jan 19 2018 14:09
yeah, it's a little confusing though
Gregg Roemhildt
@roemhildtg
Jan 19 2018 14:09
I just thought of another question I had. when do you use/not use commas in the helper args?
Or does it not matter
Matthew Phillips
@matthewp
Jan 19 2018 14:12
commas are for separate arguments
so if a helper takes 2 args (like any js function) you'd use them
what you're doing in the above example is creating an object
routeUrl(page=tournaments tournamentId=2) is equivalent to routeUrl({page: 'tournaments', tournamentId: 2}) in js
oh, I think tournaments needs to be a string there (in the stache version)
Gregg Roemhildt
@roemhildtg
Jan 19 2018 14:17
Okay. Cool, that makes sense. So any non comma separated arguments will be converted to an object
So, landscaper doesn't like the major branch for some reason...getting roemhildtg/canjs: Could not checkout non-existent branch "major"
Matthew Phillips
@matthewp
Jan 19 2018 14:19
I think you'll need to pull it from canjs/canjs first
i'm not such a git expert so I can't help a whole lot here :)
Gregg Roemhildt
@roemhildtg
Jan 19 2018 14:21
lol yeah me neither. Not sure how I can tell github to pull the major repo...it appears on the fork: https://github.com/roemhildtg/canjs/tree/major
Matthew Phillips
@matthewp
Jan 19 2018 14:22
did you try git fetch --all before pulling?
Gregg Roemhildt
@roemhildtg
Jan 19 2018 14:23
well, landscaper doesn't work with local repos, does it? You have to use it on github right?
Matthew Phillips
@matthewp
Jan 19 2018 14:23
oh
i don't know a lot about landscaper
Gregg Roemhildt
@roemhildtg
Jan 19 2018 15:12
I just used jscodeshift in the end. The transforms happen so much quicker for some reason. I suppose because landscaper is cloning/modifying/and pushing.
Kevin Phillips
@phillipskevin
Jan 19 2018 15:13
That's fine. I really just mentioned Landscaper because we'll want to use it when we modify all of the can-* repos.
Gregg Roemhildt
@roemhildtg
Jan 19 2018 15:13
Might be useful for other updates like let and arrow functions
Kevin Phillips
@phillipskevin
Jan 19 2018 15:13
so we don't have to run jscodeshift 80 times
Gregg Roemhildt
@roemhildtg
Jan 19 2018 15:13
Yeah, that doesn't sound like fun
Kevin Phillips
@phillipskevin
Jan 19 2018 15:40
added some comments to your PR @roemhildtg :smile:
Gregg Roemhildt
@roemhildtg
Jan 19 2018 16:02
Haha, just a few :) I knew there'd be lots of minor things to cleanup
Chasen Le Hara
@chasenlehara
Jan 19 2018 16:32
Our bi-weekly contributors meeting will start in just a few minutes! https://www.youtube.com/watch?v=geAmze1oFHY
RyanMilligan
@RyanMilligan
Jan 19 2018 19:41
Hello, I'm running into a problem with can-reflect, is this the right place to ask about that?
Kevin Phillips
@phillipskevin
Jan 19 2018 19:42
Yep
RyanMilligan
@RyanMilligan
Jan 19 2018 19:45
Great. Basically, we're trying to use deepAssign to set a property chain that includes an object in an array.
So I have a target that looks like this: {prop1: [{prop2: 'first'}, {prop2: 'second'}]}
And a source that looks like this: {prop1: {1: {prop2: 'change'}}}
And I want it to update the prop2 property on the second item in the target array.
The thing is, that doesn't work because assignList() starts by calling toArray(), which converts {1: {prop: 'second'}} to [{prop: 'second'}].
Note that toArray() ignores the keys on the source object, so even passing something like [undefined, {prop2: 'second'}] wouldn't work because it would still end up being the first object in the actual list it used, and I believe it would actually end up deleting the second item in the target list.
Anyway, I added a check to the top of toArray() that just returns obj if it's not list-like, and it actually worked perfectly.
But of course, in order to check that in I'd have to fork can-reflect, which we'd really rather not do. So I guess the question is, is there a better way to do this? Or would it be possible to get toArray() changed so it respects the keys of the source object?
Kevin Phillips
@phillipskevin
Jan 19 2018 19:53
hmm, I don't know off the top of my head
can you create a JSBin showing what happens vs what you expected to happen?
there's a link at the top of gitter that you can use as a starter
can.Reflect is available
as a global in that JSBin
RyanMilligan
@RyanMilligan
Jan 19 2018 19:54
Yes, let me give that a shot.
RyanMilligan
@RyanMilligan
Jan 19 2018 20:07
It has two buttons, with a label next to each. The intent is that clicking either button would change the label next to it, but in fact they both change the first because shapeReflections.toArray() ignores the keys on source objects.
RyanMilligan
@RyanMilligan
Jan 19 2018 20:21
For a little bit more background, we have a home-grown set() implementation to go along with can-util/get, and we implemented it by splitting the property path on periods and then reducing it to an object that simulated the structure, then calling deepAssign(). I just rewrote it to just reduce the original object down to the final value and set the property directly, so this isn't blocking me anymore, but I still think you should consider looking at a fix for this. Right now, deepAssign() doesn't correctly handle making changes to lists unless there's an object in the source list for each instance in the target.
Gregg Roemhildt
@roemhildtg
Jan 19 2018 21:26
when clicking route links in donejs with pushstate enabled, it causes a "navigation to page" and reloads the entire application. How can I prevent this?
Kevin Phillips
@phillipskevin
Jan 19 2018 21:48
@RyanMilligan this demonstrates your issue, right? https://jsbin.com/suyowov/edit?js,output
RyanMilligan
@RyanMilligan
Jan 19 2018 21:54
Yes, that's a much simpler repro.
Kevin Phillips
@phillipskevin
Jan 19 2018 22:03
ok, definitely looks confusing and I'm not sure why it would work that way
would you mind opening an issue?
RyanMilligan
@RyanMilligan
Jan 19 2018 22:03
I'd be happy to. Can you point me to your issue repository and any guidelines you might have for them?
Kevin Phillips
@phillipskevin
Jan 19 2018 22:04
https://github.com/canjs/can-define/issues/new I think is the best place for it
with normal arrays it works "correctly"
RyanMilligan
@RyanMilligan
Jan 19 2018 22:04
Incidentally, I did implement a solution, but it involved a change in shape.js so I can't really apply it without forking.
Kevin Phillips
@phillipskevin
Jan 19 2018 22:04
there should be a template already in the issue
it doesn't hurt to put in a PR
the worst that could happen is we'll ask you to fix it a different way and you'll have to do more work :smile:
RyanMilligan
@RyanMilligan
Jan 19 2018 22:06
Haha, perfectly tolerable. Do I just link the PR in the issue?
Kevin Phillips
@phillipskevin
Jan 19 2018 22:06
yeah... or the other way
either way works
(other way meaning link to the issue in the PR)
RyanMilligan
@RyanMilligan
Jan 19 2018 22:06
Gotcha. Ok, I'll try to get the request in and the issue logged today, thanks for your time.
Kevin Phillips
@phillipskevin
Jan 19 2018 22:06
awesome
@roemhildtg might need some more info on your routes to answer your question
can you create a sample app that shows the problem?
Kevin Phillips
@phillipskevin
Jan 19 2018 22:12
that shouldn't happen... if you click a link on http://www.place-my-order.com/ you can see only what is needed for the new route is requested
Gregg Roemhildt
@roemhildtg
Jan 19 2018 22:38

@phillipskevin The routes are quite simple. like this:

route('{page}', {page: 'home'});
route('{page}/{id}');

Page controls the current component displayed in the app body, and id is used by some but not all of the page-components depending on what page it is. For instance home doesn't use the id but admin-users/5 shows the user with id 5
I'll see if I can get a demo to you next week sometime.

and if the route is currently admin-users/ the list of users should display
RyanMilligan
@RyanMilligan
Jan 19 2018 22:44
@phillipskevin Logged canjs/can-define#308, which links to the PR. I went ahead and used your sample.