Skip to content

Commit

Permalink
[tech debt] Convert Promise syntax to modern async/await
Browse files Browse the repository at this point in the history
- remove separate async `resolveOptionsLoader` fn - for some reason this separate method causes the EuiSelectableList to remount completely, no idea why :T

+ add missing clearTimeout for scheduled cache wipe, which could potentially cause stale errors after unmount
+ add missing test case for `cache` config
  • Loading branch information
cee-chen committed Aug 30, 2024
1 parent 96d5b68 commit 0733b47
Show file tree
Hide file tree
Showing 2 changed files with 123 additions and 70 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -502,4 +502,66 @@ describe('FieldValueSelectionFilter', () => {
.eq(0)
.should('have.attr', 'title', 'Bug');
});

it('caches options if options is a function and config.cache is set', () => {
// Note: cy.clock()/cy.tick() doesn't currently work in Cypress component testing :T
// We should use that instead of cy.wait once https://github.com/cypress-io/cypress/issues/28846 is fixed
const props: FieldValueSelectionFilterProps = {
index: 0,
onChange: () => {},
query: Query.parse(''),
config: {
type: 'field_value_selection',
field: 'tag',
name: 'Tag',
cache: 5000, // Cache the loaded tags for 5 seconds
options: () =>
new Promise((resolve) => {
setTimeout(() => {
resolve(staticOptions);
}, 1000); // Spoof 1 second load time
}),
},
};
cy.spy(props.config, 'options');

const reducedTimeout = { timeout: 10 };
const assertIsLoading = (expected?: Function) => {
cy.get('.euiSelectableListItem', reducedTimeout).should('have.length', 0);
cy.get('[data-test-subj="euiSelectableMessage"]', reducedTimeout)
.should('have.text', 'Loading options')
.then(() => {
expected?.();
});
};
const assertIsLoaded = (expected?: Function) => {
cy.get('.euiSelectableListItem', reducedTimeout).should('have.length', 3);
cy.get('[data-test-subj="euiSelectableMessage"]', reducedTimeout)
.should('not.exist')
.then(() => {
expected?.();
});
};

cy.mount(<FieldValueSelectionFilter {...props} />);
cy.get('button').click();
assertIsLoading();

// Wait out the async options loader
cy.wait(1000);
assertIsLoaded(() => expect(props.config.options).to.be.calledOnce);

// Close and re-open the popover
cy.get('button').click();
cy.get('button').click();

// Cached options should immediately repopulate
assertIsLoaded(() => expect(props.config.options).to.be.calledOnce);

// Wait out the remainder of the cache, loading state should initiate again
cy.get('button').click();
cy.wait(5000);
cy.get('button').click();
assertIsLoading(() => expect(props.config.options).to.be.calledTwice);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -87,6 +87,8 @@ export class FieldValueSelectionFilter extends Component<
FieldValueSelectionFilterProps,
State
> {
cacheTimeout: ReturnType<typeof setTimeout> | undefined;

constructor(props: FieldValueSelectionFilterProps) {
super(props);
const { options } = props.config;
Expand Down Expand Up @@ -125,83 +127,68 @@ export class FieldValueSelectionFilter extends Component<
});
}

loadOptions() {
const loader = this.resolveOptionsLoader();
loadOptions = async () => {
let loadedOptions: FieldValueOptionType[];
this.setState({ options: null, error: null });
loader()
.then((options) => {
const items: {
on: FieldValueOptionType[];
off: FieldValueOptionType[];
rest: FieldValueOptionType[];
} = {
on: [],
off: [],
rest: [],
};

const { query, config } = this.props;

if (options) {
options.forEach((op) => {
const optionField = op.field || config.field;
if (optionField) {
const clause =
this.multiSelect === 'or'
? query.getOrFieldClause(optionField, op.value)
: query.getSimpleFieldClause(optionField, op.value);
const checked = this.resolveChecked(clause);
if (!checked) {
items.rest.push(op);
} else if (checked === 'on') {
items.on.push(op);
} else {
items.off.push(op);
}
}
return;
});
}

this.setState({
error: null,
activeItemsCount: items.on.length,
options: {
unsorted: options,
sorted: [...items.on, ...items.off, ...items.rest],
},
});
})
.catch(() => {
this.setState({ options: null, error: 'Could not load options' });
});
}

resolveOptionsLoader: () => OptionsLoader = () => {
const options = this.props.config.options;
if (isArray(options)) {
return () => Promise.resolve(options);
const { options, cache } = this.props.config;
try {
if (isArray(options)) {
// Synchronous options, already loaded
loadedOptions = options;
} else {
// Async options loader fn, potentially with a cache
loadedOptions = this.state.cachedOptions ?? (await options());

// If a cache time is set, populate the cache and schedule a cache reset
if (cache != null && cache > 0) {
this.setState({ cachedOptions: loadedOptions });
this.cacheTimeout = setTimeout(() => {
this.setState({ cachedOptions: null });
}, cache);
}
}
} catch {
return this.setState({ options: null, error: 'Could not load options' });
}

return () => {
const cachedOptions = this.state.cachedOptions;
if (cachedOptions) {
return Promise.resolve(cachedOptions);
}
const items: Record<string, FieldValueOptionType[]> = {
on: [],
off: [],
rest: [],
};

return (options as OptionsLoader)().then((opts) => {
// If a cache time is set, populate the cache and also schedule a
// cache reset.
if (this.props.config.cache != null && this.props.config.cache > 0) {
this.setState({ cachedOptions: opts });
setTimeout(() => {
this.setState({ cachedOptions: null });
}, this.props.config.cache);
}
const { query, config } = this.props;

return opts;
if (loadedOptions) {
loadedOptions.forEach((op) => {
const optionField = op.field || config.field;
if (optionField) {
const clause =
this.multiSelect === 'or'
? query.getOrFieldClause(optionField, op.value)
: query.getSimpleFieldClause(optionField, op.value);
const checked = this.resolveChecked(clause);
if (!checked) {
items.rest.push(op);
} else if (checked === 'on') {
items.on.push(op);
} else {
items.off.push(op);
}
}
return;
});
};
}

this.setState({
error: null,
activeItemsCount: items.on.length,
options: {
unsorted: loadedOptions,
sorted: [...items.on, ...items.off, ...items.rest],
},
});
};

resolveOptionName(option: FieldValueOptionType) {
Expand Down Expand Up @@ -264,6 +251,10 @@ export class FieldValueSelectionFilter extends Component<
if (this.props.query !== prevProps.query) this.loadOptions();
}

componentWillUnmount() {
clearTimeout(this.cacheTimeout);
}

render() {
const { query, config } = this.props;

Expand Down

0 comments on commit 0733b47

Please sign in to comment.