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.flat(Map) with some basic tests #240

Merged
merged 6 commits into from
Jan 11, 2021

Conversation

mstoykov
Copy link
Contributor

@mstoykov mstoykov commented Jan 8, 2021

No description provided.

@mstoykov
Copy link
Contributor Author

mstoykov commented Jan 8, 2021

I will be making some more as part of our try to drop corejs completely.

The list so far is :

  • Array.prototype.flat(Map) - this PR
  • String.prototype.trimLeft/Right
  • String.prototype.matchAll
  • Object.prototype.getOwnPropertyDescriptors
  • Object.prototype.values/entries

Possibly (although I hope not):

  • Object.prototype.defineSetter/defineGetter and the lookup part

Copy link
Owner

@dop251 dop251 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for submitting. I have added a couple of minor changes, otherwise looks good to me.
The only thing is most array methods have optimisations for simple arrays (see checkStdArray) where property access can be replaced with operating directly on a.values. It's not necessary though and could be added at a later stage should there be a need...

mstoykov and others added 4 commits January 11, 2021 11:22
This is exactly as the specification says it needs to be ... but I
either glossed over it or fixed it because of how I understood it should
work :(
@mstoykov
Copy link
Contributor Author

After the latest changes the only flat* test that are failing are :

--- FAIL: TestTC39 (4.22s)
    --- FAIL: TestTC39/test262 (4.22s)
        --- FAIL: TestTC39/test262/test/built-ins/Array/prototype/flat/null-undefined-elements.js (0.00s)
        Error Trace:    tc39_test.go:219
                        tc39_test.go:233
                        tc39_test.go:280
                        tc39_test.go:392
                        tc39_test.go:515
                        tc39_norace_test.go:11
        Error:          Should be empty, but was [test/built-ins/Array/prototype/flat/null-undefined-elements.js TypeError: Cannot convert undefined or null to object at flat (native)]: %!v(MISSING)
        Test:           TestTC39/test262/test/built-ins/Array/prototype/flat/null-undefined-elements.js
        --- FAIL: TestTC39/test262/test/built-ins/Array/prototype/flatMap/this-value-ctor-object-species-bad-throws.js (0.45s)
        Error Trace:    tc39_test.go:219
                        tc39_test.go:233
                        tc39_test.go:280
                        tc39_test.go:392
                        tc39_test.go:515
                        tc39_norace_test.go:11
        Error:          Should be empty, but was [test/built-ins/Array/prototype/flatMap/this-value-ctor-object-species-bad-throws.js Test262Error: throw TypeError if @@species is a number Expected a TypeError to be thrown but no exception was thrown at all at harness/sta.js:22:9(49)]: %!v(MISSING)
        Test:           TestTC39/test262/test/built-ins/Array/prototype/flatMap/this-value-ctor-object-species-bad-throws.js
        --- FAIL: TestTC39/test262/test/built-ins/Array/prototype/flatMap/this-value-ctor-object-species-custom-ctor-poisoned-throws.js (0.17s)
        Error Trace:    tc39_test.go:219
                        tc39_test.go:233
                        tc39_test.go:280
                        tc39_test.go:392
                        tc39_test.go:515
                        tc39_norace_test.go:11
        Error:          Should be empty, but was [test/built-ins/Array/prototype/flatMap/this-value-ctor-object-species-custom-ctor-poisoned-throws.js Test262Error: Return abrupt completion from species custom ctor Expected a Test262Error to be thrown but no exception was thrown at all at harness/sta.js:22:9(49)]: %!v(MISSING)
        Test:           TestTC39/test262/test/built-ins/Array/prototype/flatMap/this-value-ctor-object-species-custom-ctor-poisoned-throws.js
        --- FAIL: TestTC39/test262/test/built-ins/Array/prototype/flatMap/this-value-ctor-object-species-custom-ctor.js (0.20s)
        Error Trace:    tc39_test.go:219
                        tc39_test.go:233
                        tc39_test.go:280
                        tc39_test.go:392
                        tc39_test.go:515
                        tc39_norace_test.go:11
        Error:          Should be empty, but was [test/built-ins/Array/prototype/flatMap/this-value-ctor-object-species-custom-ctor.js Test262Error: returned value is an instance of custom ctor at harness/sta.js:22:9(49)]: %!v(MISSING)
        Test:           TestTC39/test262/test/built-ins/Array/prototype/flatMap/this-value-ctor-object-species-custom-ctor.js
        --- FAIL: TestTC39/test262/test/built-ins/Array/prototype/flatMap/this-value-ctor-object-species.js (0.53s)
        Error Trace:    tc39_test.go:219
                        tc39_test.go:233
                        tc39_test.go:280
                        tc39_test.go:392
                        tc39_test.go:515
                        tc39_norace_test.go:11
        Error:          Should be empty, but was [test/built-ins/Array/prototype/flatMap/this-value-ctor-object-species.js Test262Error: got species once Expected SameValue(«0», «1») to be true at harness/sta.js:22:9(49)]: %!v(MISSING)
        Test:           TestTC39/test262/test/built-ins/Array/prototype/flatMap/this-value-ctor-object-species.js
FAIL
exit status 1

For the record this was run through k6 which also employs babel to get some additional ES support including template variables.

I don't think I can do much about any of those?

but maybe I should now write the fast path? Including moving the check for the map function outside the loop and having 2 loops?

Co-authored-by: Dmitry Panov <[email protected]>
@dop251 dop251 merged commit 8b568f3 into dop251:master Jan 11, 2021
@mstoykov mstoykov deleted the implementFlat branch January 11, 2021 15:39
tsedgwick pushed a commit to mongodb-forks/goja that referenced this pull request Apr 14, 2021
* Implement Array.prototype.flat(Map) with some basic tests

* Apply suggestions from code review

Co-authored-by: Dmitry Panov <[email protected]>

* Add flat/flatMap to unscopables

* Don't call the map function on the already mapped elements

This is exactly as the specification says it needs to be ... but I
either glossed over it or fixed it because of how I understood it should
work :(

* Check for property existance as string

This is so
https://github.com/tc39/test262/blob/42bf3a9f7aed3790dc1d829f290dee033df41c5b/test/built-ins/Array/prototype/flat/proxy-access-count.js
passes and also because this is what the spec says

* Update builtin_array.go

Co-authored-by: Dmitry Panov <[email protected]>

Co-authored-by: Dmitry Panov <[email protected]>
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