Skip to content

feat: make result of StaticArray#slice and StaticArray#concat as instance generics. Deprecate static methods #2404

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

Merged
merged 9 commits into from
Aug 15, 2022

Conversation

MaxGraey
Copy link
Member

@MaxGraey MaxGraey commented Aug 4, 2022

StaticArray.slice / StaticArray.concat are quite not standard. Also, many users more expect StaticArray as returning type instead Array for slice method. With this change, we preserve old behavior, so it's non breakable change.

Open question:
What if change default return type from Array to StaticArray and remove StaticArray.slice at all? This will be breaking change than.

  • I've read the contributing guidelines
  • I've added my name and email to the NOTICE file

@MaxGraey MaxGraey requested a review from dcodeIO August 4, 2022 11:06
@dcodeIO
Copy link
Member

dcodeIO commented Aug 4, 2022

Iirc the idea here is that StaticArray#slice returns an Array while StaticArray.slice works with StaticArray, like other instance/static methods, i.e. the static ones exist to provide what would otherwise be missing.

@MaxGraey
Copy link
Member Author

MaxGraey commented Aug 4, 2022

The main problem with StaticArray.slice that it's non-standard nature and different signature compare to Array/SliceArray#slice (What makes refactoring difficult) and force people to spend a lot of time in docs. Also sometimes users thinking it's a bug. With new approach user will show signature suggestion. Also with inferring by returned type you will even not need specify generic type like in this case let copy: StaticArray<i32> = origStaticArr.slice()

@dcodeIO
Copy link
Member

dcodeIO commented Aug 4, 2022

I guess the same would apply to other static methods as well then, and without inference of return types seems tricky. Hmm. Sure it wouldn't be better to defer until we can update all of this according to your suggestion?

@MaxGraey
Copy link
Member Author

MaxGraey commented Aug 4, 2022

I think we can move towards this iteratively. These changes don't break anything. And then we can remove all static methods that duplicate instance methods but only with a different return type. I just don't know how soon we will have inferring returned generic types

@MaxGraey
Copy link
Member Author

MaxGraey commented Aug 4, 2022

Btw I tried do the same for StringArray.concat but due to some issue with inferring it seems we should delay this change:

ERROR TS2322: Type '~lib/array/Array<~lib/string/String>' is not assignable to type '~lib/array/Array<usize>'.
       :
   118 │ result = source.concat(["foo"]);
       │          ~~~~~~~~~~~~~~~~~~~~~~
       └─ in std/staticarray.ts(118,12)

For new signature:

concat<U extends ArrayLike<T> = Array<T>>(other: U): U {
  ....
}

UPD:
Fill an issue for that.

@MaxGraey MaxGraey changed the title feat: make result of StaticArray#slice as generic. Deprecate static StaticArray.slice feat: make result of StaticArray#slice and StaticArray#concat as instance generics. Deprecate static methods Aug 4, 2022
@MaxGraey MaxGraey requested a review from dcodeIO August 4, 2022 13:59
@MaxGraey MaxGraey merged commit 847dbde into AssemblyScript:main Aug 15, 2022
@MaxGraey MaxGraey deleted the static-array-slice branch August 15, 2022 16:19
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