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

PRO-6477 undhandled promise errors #4700

Merged
merged 17 commits into from
Sep 3, 2024
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
* Registering duplicate icon is no longer breaking the build.
* Fix widget focus state so that the in-context Add Content menu stays visible during animation
* Fix UI of areas in schemas so that their context menus are layered overtop sibling schema fields UI
* Fix unhandled promise rejections and guard against potential memory leaks, remove 3rd party `debounce-async` dependency

### Adds

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
</template>

<script>
import { debounce } from 'Modules/@apostrophecms/ui/utils';
import debounce from 'lodash/debounce';
import { Cropper } from 'vue-advanced-cropper';
import 'vue-advanced-cropper/dist/style.css';

Expand Down Expand Up @@ -129,6 +129,10 @@ export default {
};
},
beforeUnmount() {
this.onScreenResizeDebounced.cancel();
this.handleCropperChangeDebounced.cancel();
this.setCropperCoordinatesDebounced.cancel();
this.updateFocalPointCoordinatesDebounced.cancel();
this.$refs.focalPoint.removeEventListener('mousedown', this.onFocalPointMouseDown);
window.removeEventListener('resize', this.onScreenResizeDebounced);
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -125,10 +125,10 @@
</template>

<script>
import { debounce } from 'Modules/@apostrophecms/ui/utils';
import { createId } from '@paralleldrive/cuid2';
import { debounceAsync } from 'Modules/@apostrophecms/ui/utils';
import AposModifiedMixin from 'Modules/@apostrophecms/ui/mixins/AposModifiedMixin';
import AposDocsManagerMixin from 'Modules/@apostrophecms/modal/mixins/AposDocsManagerMixin';
import { createId } from '@paralleldrive/cuid2';

const DEBOUNCE_TIMEOUT = 500;

Expand All @@ -143,6 +143,7 @@ export default {
emits: [ 'archive', 'save', 'search', 'piece-relationship-query' ],
data() {
return {
mounted: false,
items: [],
isFirstLoading: true,
isLoading: false,
Expand All @@ -166,7 +167,7 @@ export default {
emoji: '🖼'
},
cancelDescription: 'apostrophe:discardImageChangesPrompt',
debouncedGetMedia: debounce(this.getMedia, DEBOUNCE_TIMEOUT),
debouncedGetMedia: null,
myovchev marked this conversation as resolved.
Show resolved Hide resolved
loadObserver: new IntersectionObserver(
this.handleIntersect,
{
Expand Down Expand Up @@ -243,6 +244,7 @@ export default {
return this.totalPages > 1 && this.currentPage === this.totalPages;
}
},

watch: {
async checked (newVal, oldVal) {
this.lastSelected = newVal.at(-1);
Expand Down Expand Up @@ -272,16 +274,31 @@ export default {
},
created() {
this.setDefaultFilters();
const debouncedGetMedia = debounceAsync(this.getMedia, DEBOUNCE_TIMEOUT);
this.debouncedGetMedia = async function (...args) {
try {
const result = await debouncedGetMedia(...args);
return result;
myovchev marked this conversation as resolved.
Show resolved Hide resolved
} catch (e) {
if (e.message !== 'debounce:canceled') {
throw e;
}
}
};
this.debouncedGetMedia.cancel = debouncedGetMedia.cancel;
myovchev marked this conversation as resolved.
Show resolved Hide resolved
},
async mounted() {
this.mounted = true;
this.modal.active = true;
await this.getMedia({ tags: true });
this.isFirstLoading = false;

apos.bus.$on('content-changed', this.onContentChanged);
apos.bus.$on('command-menu-manager-close', this.confirmAndCancel);
},
unmounted() {
beforeUnmount() {
this.mounted = false;
this.debouncedGetMedia.cancel();
myovchev marked this conversation as resolved.
Show resolved Hide resolved
apos.bus.$off('content-changed', this.onContentChanged);
apos.bus.$off('command-menu-manager-close', this.confirmAndCancel);
},
Expand All @@ -296,7 +313,10 @@ export default {
editorModified (val) {
this.modified = val;
},
async getMedia (options = {}) {
async getMedia(options = {}) {
if (!this.mounted) {
return;
}
const qs = {
...this.filterValues,
page: this.currentPage,
Expand Down Expand Up @@ -329,6 +349,9 @@ export default {
draft: true
}
));
if (!this.mounted) {
return;
}

if (options.tags) {
if (filtered) {
Expand All @@ -344,6 +367,9 @@ export default {
draft: true
}
));
if (!this.mounted) {
return;
}
myovchev marked this conversation as resolved.
Show resolved Hide resolved
this.tagList = apiResponse.choices._tags;
} else {
this.tagList = apiResponse.choices ? apiResponse.choices._tags : [];
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@
disableUnchecked: maxReached()
}"
@select-click="selectAll"
@search="onSearch"
@search="onSearchSafe"
@page-change="updatePage"
@filter="filter"
@batch="handleBatchAction"
Expand Down Expand Up @@ -129,7 +129,7 @@ import AposDocsManagerMixin from 'Modules/@apostrophecms/modal/mixins/AposDocsMa
import AposModifiedMixin from 'Modules/@apostrophecms/ui/mixins/AposModifiedMixin';
import AposPublishMixin from 'Modules/@apostrophecms/ui/mixins/AposPublishMixin';
import { useModalStore } from 'Modules/@apostrophecms/ui/stores/modal';
import { debounce } from 'Modules/@apostrophecms/ui/utils';
import { debounceAsync } from 'Modules/@apostrophecms/ui/utils';

export default {
name: 'AposDocsManager',
Expand All @@ -149,6 +149,7 @@ export default {
emits: [ 'archive' ],
data() {
return {
mounted: false,
modal: {
active: false,
triggerFocusRefresh: 0,
Expand Down Expand Up @@ -229,7 +230,9 @@ export default {
},
created() {
const DEBOUNCE_TIMEOUT = 500;
this.onSearch = debounce(this.search, DEBOUNCE_TIMEOUT);
// "mounted" in meaning of "able to mutate the state".
this.mounted = true;
this.onSearch = debounceAsync(this.search, DEBOUNCE_TIMEOUT);

this.moduleOptions.filters.forEach(filter => {
this.filterValues[filter.name] = filter.def;
Expand All @@ -252,6 +255,10 @@ export default {
apos.bus.$on('command-menu-manager-create-new', this.create);
apos.bus.$on('command-menu-manager-close', this.confirmAndCancel);
},
onBeforeUnmount() {
this.mounted = false;
this.onSearch.cancel();
},
unmounted() {
this.destroyShortcuts();
apos.bus.$off('content-changed', this.onContentChanged);
Expand Down Expand Up @@ -341,6 +348,9 @@ export default {
),
page: this.currentPage
});
if (!this.mounted) {
return;
}
myovchev marked this conversation as resolved.
Show resolved Hide resolved
myovchev marked this conversation as resolved.
Show resolved Hide resolved

this.currentPage = currentPage;
this.totalPages = pages;
Expand All @@ -350,6 +360,9 @@ export default {
},
async getAllPiecesTotal () {
const { count: total } = await this.request({ count: 1 });
if (!this.mounted) {
return;
}

this.setAllPiecesSelection({
isSelected: false,
Expand Down Expand Up @@ -379,6 +392,9 @@ export default {
}
},
async search(query) {
if (!this.mounted) {
return;
}
if (query) {
this.queryExtras.autocomplete = query;
} else if ('autocomplete' in this.queryExtras) {
Expand Down Expand Up @@ -509,6 +525,17 @@ export default {
this.checked = this.checked.filter(checkedId => doc._id !== checkedId);
}
}
},

async onSearchSafe(...args) {
try {
const result = await this.onSearch(...args);
return result;
} catch (error) {
if (error.message !== 'debounce:canceled') {
throw error;
}
}
}
}
};
Expand Down
28 changes: 20 additions & 8 deletions modules/@apostrophecms/schema/ui/apos/logic/AposInputSlug.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,18 @@
// NOTE: This is a temporary component, copying AposInputString. Base modules
// already have `type: 'slug'` fields, so this is needed to avoid distracting
// errors.
import AposInputMixin from 'Modules/@apostrophecms/schema/mixins/AposInputMixin';
import sluggo from 'sluggo';
import debounce from 'debounce-async';
import { klona } from 'klona';
import sluggo from 'sluggo';
import AposInputMixin from 'Modules/@apostrophecms/schema/mixins/AposInputMixin';
import { debounceAsync } from 'Modules/@apostrophecms/ui/utils';

export default {
name: 'AposInputSlug',
mixins: [ AposInputMixin ],
emits: [ 'return' ],
data() {
return {
mounted: false,
conflict: false,
isArchived: null,
originalParentSlug: ''
Expand Down Expand Up @@ -102,12 +103,22 @@ export default {
}
},
async mounted() {
this.debouncedCheckConflict = debounce(() => this.checkConflict(), 250);
this.mounted = true;
this.debouncedCheckConflict = debounceAsync(() => this.checkConflict(), 250);
if (this.next.length) {
await this.debouncedCheckConflict();
try {
await this.debouncedCheckConflict();
} catch (e) {
if (e.message !== 'debounce:canceled') {
throw e;
}
}
}
this.originalParentSlug = this.getParentSlug(this.next);
},
onBeforeUnmount() {
this.debouncedCheckConflict.cancel();
},
methods: {
getParentSlug(slug = '') {
return slug.slice(-1) === '/'
Expand All @@ -120,9 +131,7 @@ export default {
try {
await this.debouncedCheckConflict();
} catch (e) {
if (e === 'canceled') {
myovchev marked this conversation as resolved.
Show resolved Hide resolved
// That's fine
} else {
if (e.message !== 'debounce:canceled') {
throw e;
}
}
Expand Down Expand Up @@ -243,6 +252,9 @@ export default {
},
draft: true
});
if (!this.mounted) {
return;
}
// Still relevant?
if (slug === this.next) {
this.conflict = false;
Expand Down
Loading
Loading