Add concat() method

[mikebronner]

New method from a recent Laravel 5.4 patch. I'm not familiar with testing in Javascript, so I have put in some initial work to what I think it should be. Would you mind helping me a bit on this PR to flesh out the testing and make sure the implementation is working as expected?

Looking forward to your feedback :)

Thanks for creating this package! I have been thinking it would be great to have a package like this for a few years. Good to see someone better than me with Javascript took the initiative. :)

[ecrmnn]

Thanks for the PR @mikebronner.

No worries, I'll help you out 😄

Do you have a link to the concat implementation?
I couldn't find it in the docs.

[mikebronner]

Absolutely: laravel/framework#19318

[ecrmnn]

Use forEach instead of a loop

[mikebronner]

The problem with forEach in Javascript is that order can be messed up, right? I think keeping order is important.

https://stackoverflow.com/questions/13600922/does-javascript-array-foreach-traverse-elements-in-ascending-order

[ecrmnn]

Needs logic for when item is of different type

• Array
• Object
• String
• etc.

[ecrmnn]

This is a port of your testConcatWithArray from the Laravel PR.
I've not tested it. You should implement this, the testConcatWithCollection and a hybrid test.

Also; what happens if the collection passed to the concat method contains several arrays?

const expected = [
  4,
  5,
  6,
  'a',
  'b',
  'c',
  'Jonny',
  'from',
  'Laroe',
  'Jonny',
  'from',
  'Laroe'
];

const firstCollection = collect([4, 5, 6]);
const secondCollection = collect(['a', 'b', 'c']);
const thirdCollection = collect([{
  who: 'Jonny'
}, {
  preposition: 'from'
}, {
  where: 'Laroe'
}]);

firstCollection
  .concat(secondCollection)
  .concat(thirdCollection)
  .concat(thirdCollection);

expect(firstCollection.count()).to.eql(12);
expect(firstCollection.all()).to.eql(expected);

[mikebronner]

Sounds good, I'll work on the port test. That test is indeed already a hybrid test, is it no? Or did you mean something else?

[ecrmnn]

Yes it is. Sorry my bad.

[mikebronner]

@ecrmnn OK, made the requested updates. Let me know what you think. I updated the code to ignore anything that is not iterable. Laravel with throw an exception, and of course the developer will catch it. Was thinking the same would apply here. This is essentially "silently failing" now, which could be considered poor practice and the developer wouldn't be aware of the problem. Any suggestions?

Also, not sure how to resolve the conflicts. I added in the missing code, but not sure what github expects in addition.

Thanks :)

[ecrmnn]

The tests are failing for me? Am I missing something?
Please make the tests pass. I'll have a look later 😄

[mikebronner]

@ecrmnn I don't know how to run the tests locally, I'm unfamiliar with JS testing. :(

[mikeerickson]

@mikebronner Sounds like a great opportunity to learn :-)

[mikebronner]

@mikeerickson Perhaps you would like to help out on this one? I don't have the bandwidth to figure it out at the moment without some guidance on what needs to be set up. A list of commands for me to run would suffice.

[mikeerickson]

@mikebronner Most definitely, I am going to create a UMD testing workflow for the project, as well as take on the task of separating files (and tests) into separate source files.

When all done, it will be a great source for you to learn from.

[faalotaiby]

If you consider the main argument that you want to add the method concat() for, it will help the method `concat() returns a copy as a shallow that contains different copies of the elements included combined from the original arrays.