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

refactor: chunk multi-message rendering to prevent singular long task #939

Merged

Conversation

grablack
Copy link
Contributor

Break up the long running tasks

@grablack grablack marked this pull request as ready for review May 19, 2023 13:40
@grablack grablack requested a review from Seavenly May 23, 2023 17:05
src/library/controllers/message/interface.js Outdated Show resolved Hide resolved
src/library/controllers/message/interface.js Outdated Show resolved Hide resolved
src/library/controllers/message/interface.js Outdated Show resolved Hide resolved
src/library/controllers/message/interface.js Outdated Show resolved Hide resolved
src/library/controllers/message/interface.js Outdated Show resolved Hide resolved
src/library/controllers/message/interface.js Outdated Show resolved Hide resolved
if (err.message === 'offer_validation_error') {
return undefined;
if (mapIndex === 0) {
return renderOrUpdateMessage();
Copy link
Collaborator

Choose a reason for hiding this comment

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

We can remove the consistent-return eslint warning by updating this slightly

Suggested change
return renderOrUpdateMessage();
renderOrUpdateMessage();
return;

Comment on lines 61 to 65
/* eslint-disable-next-line promise/no-native, compat/compat */
Promise.all(
validContainers.map((container, mapIndex) => {
/* eslint-disable-next-line promise/no-native, compat/compat, consistent-return */
return new Promise((resolve, reject) => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Based on the previous comment we can't make this change?

Suggested change
/* eslint-disable-next-line promise/no-native, compat/compat */
Promise.all(
validContainers.map((container, mapIndex) => {
/* eslint-disable-next-line promise/no-native, compat/compat, consistent-return */
return new Promise((resolve, reject) => {
ZalgoPromise.all(
validContainers.map((container, mapIndex) => {
return new ZalgoPromise((resolve, reject) => {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When I removed the promises for zalgopromise I forgot to add back the new ZalgoPromise((resolve, reject) portion. Thats why I couldn't get it to return

@Seavenly Seavenly changed the title refactor: multiple-message rendering (DTCRCMERC-2240) refactor: chunk multi-message rendering to prevent singular long task Jun 6, 2023
@JustinDoan JustinDoan merged commit 9b1a240 into paypal:develop Jun 14, 2023
@github-actions
Copy link

🎉 This PR is included in version 1.43.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

github-actions bot pushed a commit that referenced this pull request Jun 14, 2023
## [1.43.0](v1.42.0...v1.43.0) (2023-06-14)

### Features

* Update UK Pi3 Legal Disclosure (DTCRCMERC-2267) ([#936](#936)) ([a186771](a186771))

### Bug Fixes

* add cookie global ([#951](#951)) ([a12ebb3](a12ebb3))

### Build System

* remove old stage bundle url ([#947](#947)) ([c669016](c669016))

### Continuous Integration

* Github action workflow npm publish ([#923](#923)) ([0e38156](0e38156))
* remove use of deprecated set-output command ([#938](#938)) ([9ef6eb0](9ef6eb0))

### Code Refactoring

* chunk multi-message rendering to prevent singular long task ([#939](#939)) ([9b1a240](9b1a240))
* remove NE from US Pay Monthly modal legal disclosure ([#906](#906)) ([eb9ffb0](eb9ffb0))
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants