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

Implement Array prototype methods #36

Closed
32 of 33 tasks
jasonwilliams opened this issue Jun 29, 2019 · 28 comments · Fixed by #1365
Closed
32 of 33 tasks

Implement Array prototype methods #36

jasonwilliams opened this issue Jun 29, 2019 · 28 comments · Fixed by #1365
Labels
builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request help wanted Extra attention is needed

Comments

@jasonwilliams
Copy link
Member

jasonwilliams commented Jun 29, 2019

Similar to #13 but for the Array

  • Array.of() (Implement Array.of #1127)
  • Array.prototype.concat()
  • Array.prototype.copyWithin()
  • Array.prototype.entries()
  • Array.prototype.every()
  • Array.prototype.fill()
  • Array.prototype.filter()
  • Array.prototype.find()
  • Array.prototype.findIndex()
  • Array.prototype.flat()
  • Array.prototype.flatMap()
  • Array.prototype.forEach()
  • Array.prototype.includes()
  • Array.prototype.indexOf()
  • Array.prototype.join()
  • Array.prototype.keys()
  • Array.prototype.lastIndexOf()
  • Array.prototype.map()
  • Array.prototype.pop()
  • Array.prototype.push()
  • Array.prototype.reduce()
  • Array.prototype.reduceRight()
  • Array.prototype.reverse()
  • Array.prototype.shift()
  • Array.prototype.slice()
  • Array.prototype.some()
  • Array.prototype.sort()
  • Array.prototype.splice()
  • Array.prototype.toLocaleString()
  • Array.prototype.toString()
  • Array.prototype.unshift()
  • Array.prototype.values()
  • Array.prototype@@iterator

Implementation happens here: https://github.com/boa-dev/boa/blob/master/boa/src/builtins/array/mod.rs

@jasonwilliams jasonwilliams added the good first issue Good for newcomers label Jun 29, 2019
@calluw
Copy link
Contributor

calluw commented Jun 30, 2019

@jasonwilliams For the array, what is a Javascript array represented as in Boa? From my investigation appears to be as an Object, so in the underlying Rust a hashmap mapping from index to value: is that accurate?

Does that mean in Boa there's no Javascript difference between arrays and objects? Is that intentional or just because Array isn't implemented yet? Should we have an "Array" entry in the ValueData enum?

@jasonwilliams
Copy link
Member Author

That's a very good point, and yes, what you said is accurate. There is no Array primitive value in JavaScript, so we can't have an array datatype in the value enum. (for now) An Array is just an Object with a prototype of Array methods.

V8 do some optimisations with arrays with a backing store: https://v8.dev/blog/fast-properties
I'm not sure we need to worry about fast property access just yet.

For now we just need to create a global constructor (Array), similar to what we do with strings.
Arrays are then objects who's prototype is set to Array.prototype.

@calluw
Copy link
Contributor

calluw commented Jun 30, 2019

I think my main questions would likely be whether in order to implement the majority of Array prototype methods we should be converting to real Rust arrays/Vectors/etc under the covers or just using hashmaps for now?

@jasonwilliams
Copy link
Member Author

jasonwilliams commented Jun 30, 2019

I would go hashmaps for now, you will hit way too many edge/corner cases if you try to convert them to Rust Vectors (i.e length shouldn't be counted). It's not the most performant but the most correct. We can fix speed later on with some optimisations.
I will give it some thought, but https://v8.dev/blog/fast-properties is worth reading through, plus https://v8.dev/blog/elements-kinds

@simonbrahan
Copy link
Contributor

I'll take a look at indexOf and lastIndexOf.

@jasonwilliams
Copy link
Member Author

@simonbrahan ok great

@IovoslavIovchev
Copy link
Contributor

IovoslavIovchev commented Oct 18, 2019

I'd like to take a look at map.

@muskuloes
Copy link
Contributor

I'll like to work on slice

@jasonwilliams
Copy link
Member Author

@IovoslavIovchev can you put yourself on (or reply to) this issue jasonwilliams#176

@muskuloes same for jasonwilliams#177

@simonbrahan
Copy link
Contributor

Can I take includes?

@Razican Razican added the enhancement New feature or request label Apr 3, 2020
@HalidOdat
Copy link
Member

HalidOdat commented Apr 12, 2020

A lot of these methods have already been implemented. Such as every, find, some and some others.
I think we should update the check list. :)

@Razican
Copy link
Member

Razican commented Apr 13, 2020

I updated the checklist. I noticed that the Array.prototype.includes() does not have documentation.

@HalidOdat
Copy link
Member

HalidOdat commented Apr 13, 2020

I updated the checklist. I noticed that the Array.prototype.includes() does not have documentation.

I will add documentation for it.

@brian-gavin
Copy link
Contributor

Hey, like String the links are out of date now.

Array can now be found here!

@Razican
Copy link
Member

Razican commented Jun 1, 2020

Hey, like String the links are out of date now.

Array can now be found here!

Yep! I updated the link, and once again, this might be blocked in #419.

@Razican Razican added the help wanted Extra attention is needed label Jun 1, 2020
@benjaminflin
Copy link
Contributor

I'd like to take a look at reduce

@Razican
Copy link
Member

Razican commented Jul 8, 2020

I'd like to take a look at reduce

Go ahead! :D

@benjaminflin
Copy link
Contributor

Also Since I did reduce, reduceRight should be pretty straightforward to implement.

@HalidOdat
Copy link
Member

@joshwd36 has implemented Array.prototype.keys(), Array.prototype.entries(), Array.prototype.values() and Array.prototype@@iterator in #704

@HalidOdat HalidOdat added the builtins PRs and Issues related to builtins/intrinsics label Oct 5, 2020
@davimiku
Copy link
Contributor

Hi all, I would like to work on Array.prototype.flat and Array.prototype.flatMap if those are available.

@HalidOdat
Copy link
Member

Go ahead @davidmikulis :)

@davimiku
Copy link
Contributor

The Pull Request for Array.prototype.flat and Array.prototype.flatMap is available at #1132 for whenever anyone has time to review. Thanks!

@jedel1043
Copy link
Member

Should Array.prototype.toSource() be implemented at all? MDN Web Docs shows that there's barely any support for this method, as it was deprecated many years ago.

@Razican
Copy link
Member

Razican commented Jun 17, 2021

Should Array.prototype.toSource() be implemented at all? MDN Web Docs shows that there's barely any support for this method, as it was deprecated many years ago.

It shouldn't be implemented, since it's not in the spec.

neeldug added a commit to neeldug/boa that referenced this issue Jun 27, 2021
- adds array.prototype.splice
- todo: fix stalls at certain testcases

Closes boa-dev#36
neeldug added a commit to neeldug/boa that referenced this issue Jul 30, 2021
- adds array.prototype.splice
- todo: fix stalls at certain testcases

Closes boa-dev#36
neeldug added a commit to neeldug/boa that referenced this issue Jul 30, 2021
- adds array.prototype.splice
- todo: fix stalls at certain testcases

Closes boa-dev#36
neeldug added a commit to neeldug/boa that referenced this issue Jul 30, 2021
- adds array.prototype.splice
- todo: fix stalls at certain testcases

Closes boa-dev#36
neeldug added a commit to neeldug/boa that referenced this issue Jul 30, 2021
- adds array.prototype.splice
- todo: fix stalls at certain testcases

Closes boa-dev#36
@jedel1043
Copy link
Member

jedel1043 commented Sep 6, 2021

We're still missing toLocaleString. Maybe we could transfer all internationalization missing features into its own issue to close this one.

@jedel1043 jedel1043 reopened this Sep 6, 2021
@Razican
Copy link
Member

Razican commented Sep 7, 2021

We're still missing toLocaleString. Maybe we could transfer all internationalization missing features into its own issue to close this one.

Yes, let's move localization yo a different issue. Is everything else done?

@jedel1043
Copy link
Member

We're still missing toLocaleString. Maybe we could transfer all internationalization missing features into its own issue to close this one.

Yes, let's move localization yo a different issue. Is everything else done?

Yep, everything else is implemented

@jedel1043
Copy link
Member

Closing in favour of #1562.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
builtins PRs and Issues related to builtins/intrinsics enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

Successfully merging a pull request may close this issue.