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

Remove TODOs, tidy up last things #3301

Merged
merged 10 commits into from
Nov 2, 2023
Merged

Remove TODOs, tidy up last things #3301

merged 10 commits into from
Nov 2, 2023

Conversation

HarelM
Copy link
Collaborator

@HarelM HarelM commented Oct 30, 2023

Hopefully the last PR agains #3233
It should remove the last things that were not removed in previous PRs.

@codecov-commenter
Copy link

codecov-commenter commented Oct 31, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (async-actor@2893ab3). Click here to learn what that means.

Additional details and impacted files
@@              Coverage Diff               @@
##             async-actor    #3301   +/-   ##
==============================================
  Coverage               ?   75.23%           
==============================================
  Files                  ?      241           
  Lines                  ?    19280           
  Branches               ?     4284           
==============================================
  Hits                   ?    14505           
  Misses                 ?     4775           
  Partials               ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@HarelM HarelM marked this pull request as ready for review October 31, 2023 13:21
@HarelM HarelM requested review from neodescis and birkskyum October 31, 2023 13:28
@HarelM
Copy link
Collaborator Author

HarelM commented Oct 31, 2023

OK, this is the last PR I think.
I've removed a lot of places that were using callbacks.
The "public" API of both ajax and actor are using promises and AbortContoller.
I've removed most of the TODOs I added while doing the refactoring (a few minor ones left do decide on which are related to changes of the public API).

I need to think what should be done about public methods that are using callbacks and if to create a 3.6 version that supports both callbacks and promises to ease the migration.
IDK...

CC: @smellyshovel, @msbarry
Once the other PR is merged to main I'll create a pre-release version I think.

@HarelM HarelM requested a review from louwers November 1, 2023 15:54
@neodescis
Copy link
Collaborator

Looks good to me. I'll be happy to beta test for you when it's ready!

@HarelM HarelM merged commit 8259409 into async-actor Nov 2, 2023
@HarelM HarelM deleted the async-actor-todos branch November 2, 2023 06:34
HarelM added a commit that referenced this pull request Nov 23, 2023
* Initial concept

* More improvements

* Fix lint

* Main commit to move most of the actor code to use promises and register. There are still things to do.

* Fix actor tests

* Remove more any when it comes to worker self typings.

* Improve more types, add more tests

* Improve more types, add more tests

* Fix lint

* Fix more tests

* Fix tests

* Fix some sytle tests

* Add comment for todo.

* Fix test related to message parameters change

* Fix remaining tests

* Added todo to accomodate for code review.

* Fix build test

* Fix geojson types

* Rename some events, remove clutter a bit

* Fix issue with multiple maps found in integration tests

* Fix tests and improve types and docs a bit more

* Final fixes to make the tests pass

* Fix build test

* a couple of tweaks (#3267)

* Add changelog item

* More improvement to async `Actor` and replace more callbacks with promises  (#3269)

* Improve vector tile source

* Fix unit tests

* Fix render tests for the case of 404

* Fix lint

* Remove unused method

* More improvements to remove callbacks and move to promises

* Fixed some tests, more fixes are needed.

* Fix remaining tests.

* Fix lint

* Fix tile parser code to pass render tests

* Fix lint

* Fix lint

* More fixes and tests in the infrasturcure when it comes to cancelling

* forgot to inclue this...

* Fix lint and build

* Convert parse to be a proper async method.

* Add more test coverate to actor, allow more testing in mock web worker.

* Replace getJson call with async code and remove more callbacks (#3273)

* Add abort between actors of main thread and worker thread with tests

* Fix lint

* Fix lint

* Fix node from lts to nvmrc

* Improve the code of vector tile and geojson workers

* Fix lint

* Add debug when tests are failing

* Do not throw in case of empty geojson tile

* Fix lint

* Move geojson worker source to be more async like.

* Change getJSON to be async extenally

* Update src/ui/map.test.ts

Co-authored-by: Bart Louwers <[email protected]>

---------

Co-authored-by: Bart Louwers <[email protected]>

* Move image queue to use promises, move getArrayBuffer to use promises (#3280)

* Migrate getArrayBuffer to be promise based

* Remove dead code in ajax.ts file

* Code review changes (#3278)

Co-authored-by: neodescis <[email protected]>

* Make image request return promises.

* Move process queue to the right location

* Increase build size

* Last fixes

* fix lint warning

* Fix lint

* update docs

Co-authored-by: neodescis <[email protected]>

* Update docs

Co-authored-by: neodescis <[email protected]>

* Fix docs

* Code review comments.

* Update src/util/image_request.ts

* Update src/util/util.ts

---------

Co-authored-by: neodescis <[email protected]>
Co-authored-by: neodescis <[email protected]>

* Remove TODOs, tidy up last things (#3301)

* Remove some TODOs

* Remove some todos, make ajax API fully promise based

* Move tileJson to be async

* fix lint

* Remove another TODO

* Move makexmlhttprequest to return a promise

* Make load_sprite async

* Added relevant tests

* Final fixes

* Update src/source/vector_tile_source.test.ts

Co-authored-by: neodescis <[email protected]>

---------

Co-authored-by: neodescis <[email protected]>

* Async actor docs additions (#3305)

* Fix missing classes in the docs

* Fixed the relevant docs in the places where API calls were changed.

* Fix lint

* Remove some "then" from tests

* remove todos, remove "then"s, simplify the ajax code a bit

* Remove as any from where it's not needed

* Fix code review expect stuff...

* explicit promises instead of array.

* Improve ajax further

* Improve docs for RTL plugin status and reduce code in geojson worker test

* Last simplification to actor, simplify a style test.

* Add missing docs comments

* Fix lint...

* Fix Actor XSS, introduce subscribe (#3329)

* Fix XSS, intorduce subscribe

* Update changelog

* Fix developer diagram with updated flow

* Async actor no log warnings and errors in unit tests (#3368)

* Upgrade jest, fix tests, remove console log messages.

* Fix incorrect additions, lint

* Async actor remove cancelables (#3371)

* Remove more places that use cancelable

* Remove more places with canceable

* Async actor callback and promise (#3374)

* Remove more places that use cancelable

* Remove more places with canceable

* callback and promise

* Improve sources code when it is related to callback

* Use callback failback when needed

* Update build size test

* Removed callbacks from various places, adding docs, simplifying tests

* Improve type

* Update src/util/actor.test.ts

Co-authored-by: neodescis <[email protected]>

* Make load async for all sources

---------

Co-authored-by: neodescis <[email protected]>

* Update changelog with most of the changes

---------

Co-authored-by: Michael Barry <[email protected]>
Co-authored-by: Bart Louwers <[email protected]>
Co-authored-by: neodescis <[email protected]>
Co-authored-by: neodescis <[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.

3 participants