Skip to content

Mark data-backed record variables async#3073

Open
jaemin-capslock wants to merge 1 commit into
microsoft:mainfrom
jaemin-capslock:minko/fix-record-variable-async
Open

Mark data-backed record variables async#3073
jaemin-capslock wants to merge 1 commit into
microsoft:mainfrom
jaemin-capslock:minko/fix-record-variable-async

Conversation

@jaemin-capslock

Copy link
Copy Markdown

Assume a case where
App.OnStart= Set(varAccount, First(Accounts)
Toggle1.Default=Coalesce(varAccount.'Option set', false)

The emitted Rule.IsAsync was false because pfx interpreted varAccount.'Option set' as a synchronous variable lookup, which depends on the datasource Accounts. As line 44 in NameLookupInfo.cs states, they should have been accessed async, hence Rule.IsAsync should have been true.

This caused issues in Powerapps. Emitted rule had IsAsync: False, so it translated that as synchronous js code. But the argument was actually async, which meant we were passing a Promise to a sync js function.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@jaemin-capslock jaemin-capslock requested a review from a team as a code owner June 8, 2026 21:59
{
if (dataSource.RequiresAsync)
{
return true;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

that's definitely not right. Almost every type from a data source will have an associated data source marked as requires async.


// Any connectedDataSourceInfo or option set or view needs to be accessed asynchronously to allow data to be loaded.
IsAsync = Data is IExternalTabularDataSource || Kind == BindKind.OptionSet || Kind == BindKind.View || isAsync;
IsAsync = Data is IExternalTabularDataSource || Kind == BindKind.OptionSet || Kind == BindKind.View || isAsync || TypeRequiresAsync(kind, type, data);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

are you sure it's not just that we're missing OptionSetValue in this list?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

or that specifically coercion from an optionset value to a string should be marked as async?

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