Add deprecation guide for array prototype extensions for V5 #1192

smilland · 2022-09-01T05:47:31Z

RFC: Deprecate array prototype extensions

It's ready for review, though only can be merge after the RFC is approved.

netlify · 2022-09-01T05:47:57Z

✅ Deploy Preview for ember-deprecations ready!

Name	Link
🔨 Latest commit	`814dfca`
🔍 Latest deploy log	https://app.netlify.com/sites/ember-deprecations/deploys/64262c22b8f7810008071392
😎 Deploy Preview	https://deploy-preview-1192--ember-deprecations.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

content/ember/v5/deprecate-array-prototype-extensions.md

tniezurawski · 2023-03-30T19:52:17Z

content/ember/v5/deprecate-array-prototype-extensions.md

+After:
+```js
+const someArray = [{ food: 'apple', isFruit: true }, { food: 'beans', isFruit: false }];
+someArray.any(el => el.isFruit); // true


Array doesn't have .any() method. I think .some() is appropriate here.

Suggested change

someArray.any(el => el.isFruit); // true

someArray.some(el => el.isFruit); // true

content/ember/v5/deprecate-array-prototype-extensions.md

tniezurawski · 2023-03-30T19:56:27Z

content/ember/v5/deprecate-array-prototype-extensions.md

+ ? a.food?.localCompare(b.food)
+ : a.isFruit - b.isFruit;
+}); // [{ food: 'apple', isFruit: true }, { food: 'beans', isFruit: false }]
+```


In general, I think that it would have to be stated here that this implementation is not as robust as Ember Array's .sortBy. It has field names hardcoded and can't take an arbitrary number of field names.

tniezurawski · 2023-03-30T20:00:07Z

content/ember/v5/deprecate-array-prototype-extensions.md

+Before:
+```js
+const someArray = [{ food: 'apple', isFruit: true }, { food: 'beans', isFruit: false }];
+someArray.toArray(); // [{ food: 'apple', isFruit: true }, { food: 'beans', isFruit: false }]
+```
+
+After:
+```js
+const someArray = [{ food: 'apple', isFruit: true }, { food: 'beans', isFruit: false }];
+[...someArray] // [{ food: 'apple', isFruit: true }, { food: 'beans', isFruit: false }]
+```


I never used this method but the docs are saying:

Simply converts the object into a genuine array. The order is not guaranteed. Corresponds to the method implemented by Prototype.

So I guess an example should show how to convert an object into an array. I'd have to check how the original function works but would this be expected?

const person = { firstName: 'John', lastName: 'Doe' }; Object.entries(person); // [ [ 'firstName', 'John' ], [ 'lastName', 'Doe' ] ]

🤔

content/ember/v5/deprecate-array-prototype-extensions.md

Co-authored-by: Tomek Nieżurawski <[email protected]>

smilland · 2023-03-31T00:41:33Z

thank you so much for reviewing @tniezurawski !

elgordino · 2023-04-03T19:38:41Z

content/ember/v5/deprecate-array-prototype-extensions.md

+
+ @action
+ pushObject(value) {
+ this.abc = [...this.abc, value];


I'm a little confused by the difference between this example and the pushObject example above which uses TrackedArray. Is it a requirement to assign to this.abc when using @tracked but not when using TrackedArray? Or is the different method shown to indicate that both approaches are valid?

yeah, @tracked will only trigger the reactivity when a new value gets assigned. When the existing native array is internally mutated, like calling .push(), then this will not be reactive. In opposite to TrackedArray, which does this kind of deep tracking. So I think the example is right.

elgordino · 2023-04-03T19:42:16Z

content/ember/v5/deprecate-array-prototype-extensions.md

+
+ @action
+ pushObjects(values) {
+ this.abc.splice(this.abc.length, 0, ...values);


Would this.abc.push(...values) work here? It would be simpler and match the first after example for pushObject above?

yeah, i think so!

simonihmig

Lots of details in that migration guide, great work @smilland! I know this has been quite a whole since you worked on this, so would you still be available to look into and work on the provided suggestions?

The RFC was accepted long ago, so I think we can and should get this merged asap, as I would like to also get the actual deprecation implemented before we miss the time window for the v6 release!

simonihmig · 2024-04-30T09:09:54Z

content/ember/v5/deprecate-array-prototype-extensions.md

+ }
+}
+
+[new Person('Tom'), new Person('Joe')].map(person => person['greet']?.('Hi')); // ['Hi Tom', 'Hi Joe']


Suggested change

[new Person('Tom'), new Person('Joe')].map(person => person['greet']?.('Hi')); // ['Hi Tom', 'Hi Joe']

[new Person('Tom'), new Person('Joe')].forEach(person => person.greet('Hi')); // ['Hi Tom', 'Hi Joe']

I think forEach is more appropriate than map here, since we don't use the value returned by map. Also the method call can be simplified as suggested I believe? invoke might do that optional chaining internally, but if people were to rewrite that code to native, I think this is what they would/should write.

simonihmig · 2024-04-30T09:16:16Z

content/ember/v5/deprecate-array-prototype-extensions.md

+ : a.isFruit - b.isFruit;
+}); // [{ food: 'apple', isFruit: true }, { food: 'beans', isFruit: false }]
+```
+#### `toArray`


Is this example even needed? I think you would use toArray an an array-like object like A(), to turn it into a native array. But here we are only dealing with native arrays, so what's the point of toArray() usage then?

simonihmig · 2024-04-30T09:18:39Z

content/ember/v5/deprecate-array-prototype-extensions.md

+const someArray = ['a', 'a', 'b', 'b']
+[...new Set(someArray)] // ['a', 'b']


This example should match the one above, so rather like:

Suggested change

const someArray = ['a', 'a', 'b', 'b']

[...new Set(someArray)] // ['a', 'b']

const someArray = [1, 2, 3, undefined, 3];

[...new Set(someArray)] // [1, 2, 3, undefined]

simonihmig · 2024-04-30T09:20:20Z

content/ember/v5/deprecate-array-prototype-extensions.md

+id: deprecate-array-prototype-extensions
+title: "Deprecate array prototype extensions"
+until: '6.0.0'
+since: '5.0.0'


Should we update this to the current one on main, assuming we get the deprecation implemented in that version?

Suggested change

since: '5.0.0'

since: '5.10.0'

simonihmig · 2024-04-30T09:24:01Z

content/ember/v5/deprecate-array-prototype-extensions.md

+import { TrackedArray } from 'tracked-built-ins';
+
+export default class SampleComponent extends Component {
+ abc = new TrackedArray();


Suggested change

abc = new TrackedArray();

abc = new TrackedArray(['x', 'y', 'z', 'x']);

simonihmig · 2024-04-30T09:28:11Z

content/ember/v5/deprecate-array-prototype-extensions.md

+
+ @action
+ pushObject(value) {
+ this.abc = [...this.abc, value];


yeah, @tracked will only trigger the reactivity when a new value gets assigned. When the existing native array is internally mutated, like calling .push(), then this will not be reactive. In opposite to TrackedArray, which does this kind of deep tracking. So I think the example is right.

simonihmig · 2024-04-30T09:29:28Z

content/ember/v5/deprecate-array-prototype-extensions.md

+
+ @action
+ pushObjects(values) {
+ this.abc.splice(this.abc.length, 0, ...values);


yeah, i think so!

simonihmig · 2024-04-30T09:31:51Z

content/ember/v5/deprecate-array-prototype-extensions.md

+ this.abc.splice(start, len);
+ this.abc = [...this.abc];


I think this would also work, and be a bit simpler?

Suggested change

this.abc.splice(start, len);

this.abc = [...this.abc];

this.abc.splice(start, len);

this.abc = this.abc;

simonihmig · 2024-04-30T09:34:51Z

content/ember/v5/deprecate-array-prototype-extensions.md

+ let loc = this.abc.length || 0;
+ while (--loc >= 0) {
+ let curValue = this.abc.at(loc);
+
+ if (curValue === value) {
+ this.abc.splice(loc, 1);
+ }
+ }


Wouldn't a simple filer call work here?

Suggested change

let loc = this.abc.length || 0;

while (--loc >= 0) {

let curValue = this.abc.at(loc);

if (curValue === value) {

this.abc.splice(loc, 1);

}

}

this.abc.filter(item => item !== value);

same for the related examples below...

hanli added 7 commits May 4, 2022 00:32

[feat] Deprecate array prototype extensions

a8721ca

Add full convenient functions

fe11c6f

Rephrase and change formatting

83ab592

Use person instead

5d654f5

Add all other method for mutableArray

86b06e2

Separate examples per metho

e0f5c20

Separate before/after per method for mutation methods

9a0da27

smilland changed the title ~~Add deprecate guide for array prototype extensions for V5~~ Add deprecation guide for array prototype extensions for V5 Sep 1, 2022

smilland mentioned this pull request Sep 22, 2022

RFC: Deprecate array prototype extensions emberjs/rfcs#848

Merged

tniezurawski reviewed Mar 30, 2023

View reviewed changes

content/ember/v5/deprecate-array-prototype-extensions.md Outdated Show resolved Hide resolved

tniezurawski reviewed Mar 30, 2023

View reviewed changes

smilland and others added 4 commits March 30, 2023 17:39

Update content/ember/v5/deprecate-array-prototype-extensions.md

3b7d96c

Co-authored-by: Tomek Nieżurawski <[email protected]>

Update content/ember/v5/deprecate-array-prototype-extensions.md

6040e0f

Co-authored-by: Tomek Nieżurawski <[email protected]>

Update content/ember/v5/deprecate-array-prototype-extensions.md

9fe50d1

Co-authored-by: Tomek Nieżurawski <[email protected]>

Update content/ember/v5/deprecate-array-prototype-extensions.md

814dfca

Co-authored-by: Tomek Nieżurawski <[email protected]>

elgordino reviewed Apr 3, 2023

View reviewed changes

simonihmig mentioned this pull request Apr 30, 2024

Advance RFC #0848 "Deprecate array prototype extensions" to Stage Ready for Release emberjs/rfcs#1020

Draft

11 tasks

simonihmig reviewed Apr 30, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add deprecation guide for array prototype extensions for V5 #1192

Add deprecation guide for array prototype extensions for V5 #1192

smilland commented Sep 1, 2022 •

edited

netlify bot commented Sep 1, 2022 •

edited

tniezurawski Mar 30, 2023

tniezurawski Mar 30, 2023

tniezurawski Mar 30, 2023

smilland commented Mar 31, 2023

elgordino Apr 3, 2023

simonihmig Apr 30, 2024

elgordino Apr 3, 2023

simonihmig Apr 30, 2024

simonihmig left a comment

simonihmig Apr 30, 2024

simonihmig Apr 30, 2024

simonihmig Apr 30, 2024

simonihmig Apr 30, 2024

simonihmig Apr 30, 2024

simonihmig Apr 30, 2024

simonihmig Apr 30, 2024

simonihmig Apr 30, 2024

simonihmig Apr 30, 2024

	someArray.any(el => el.isFruit); // true
	someArray.some(el => el.isFruit); // true

	[new Person('Tom'), new Person('Joe')].map(person => person['greet']?.('Hi')); // ['Hi Tom', 'Hi Joe']
	[new Person('Tom'), new Person('Joe')].forEach(person => person.greet('Hi')); // ['Hi Tom', 'Hi Joe']

		const someArray = ['a', 'a', 'b', 'b']
		[...new Set(someArray)] // ['a', 'b']

	abc = new TrackedArray();
	abc = new TrackedArray(['x', 'y', 'z', 'x']);

Add deprecation guide for array prototype extensions for V5 #1192

Are you sure you want to change the base?

Add deprecation guide for array prototype extensions for V5 #1192

Conversation

smilland commented Sep 1, 2022 • edited

netlify bot commented Sep 1, 2022 • edited

✅ Deploy Preview for ember-deprecations ready!

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smilland commented Mar 31, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonihmig left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

smilland commented Sep 1, 2022 •

edited

netlify bot commented Sep 1, 2022 •

edited