feat(fluid-build): Per package Declarative Tasks #24100

curtisman · 2025-03-19T23:13:28Z

Enable declarative task to be specified per package.
Fix hanging spinner if isUpToDate() throw an exception by returning false if it happens.

curtisman · 2025-03-19T23:13:43Z

Copilot

Pull Request Overview

This PR introduces per-package declarative tasks, allowing task definitions to be specified at the package level. It also fixes a hanging spinner issue by returning false when isUpToDate throws an exception.

Wraps the isUpToDate check in a try-catch block to prevent hanging when an exception occurs.
Enables lookup of declarative tasks from both package-specific and global fluidBuild configurations.
Updates error message formatting in task creation for concurrently executed scripts.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
build-tools/packages/build-tools/src/fluidBuild/buildGraph.ts	Added try-catch in isUpToDate to avoid hanging builds on errors.
build-tools/packages/build-tools/src/fluidBuild/tscUtils.ts	Added a try-catch around require.resolve for typescript, ensuring a clear error on failure.
build-tools/packages/build-tools/src/fluidBuild/tasks/taskFactory.ts	Updated getTaskForExecutable signature and refined error message formatting in concurrently command handling.

Comments suppressed due to low confidence (1)

build-tools/packages/build-tools/src/fluidBuild/tasks/taskFactory.ts:144

[nitpick] Consider unifying the error message formatting for concurrently commands to ensure consistency with similar error messages across the codebase.

`${node.pkg.nameColored}: Unable to find any tasks listed in 'concurrently' command${taskName ? ` '${taskName}'` : ""}`

build-tools/packages/build-tools/src/fluidBuild/buildGraph.ts

build-tools/packages/build-tools/src/fluidBuild/tscUtils.ts

tylerbutler · 2025-04-07T22:34:55Z

/azp run Build - protocol-definitions,Build - test-tools,server-gitrest,server-gitssh,server-historian,server-routerlicious,Build - client packages,repo-policy-check

azure-pipelines · 2025-04-07T22:35:09Z

Azure Pipelines successfully started running 1 pipeline(s).

tylerbutler · 2025-04-07T22:35:16Z

/azp run Build - api-markdown-documenter,Build - benchmark-tool,Build - build-common,Build - build-tools,Build - common-utils,Build - eslint-config-fluid,Build - eslint-plugin-fluid

azure-pipelines · 2025-04-07T22:35:31Z

Azure Pipelines successfully started running 1 pipeline(s).

tylerbutler · 2025-04-07T22:37:48Z

@curtisman Can you help me understand the use-case? Is there some reason declaring a global task in not sufficient?

tylerbutler · 2025-04-07T22:42:26Z

build-tools/packages/build-tools/src/fluidBuild/tasks/taskFactory.ts

+		node.pkg.packageJson.fluidBuild?.declarativeTasks?.[executable] ??
+		declarativeTasks?.[executable];


What if a declarative task is defined in the root of the workspace? How does it interact with the ones declared in the config and those in the individual packages?

For declarative task, it is an override. (which match task dependencies behavior, where the project specific config is an override (except if the override say to add to the global ones.)

tylerbutler · 2025-04-07T22:42:30Z

build-tools/packages/build-tools/src/fluidBuild/buildGraph.ts

+			return isUpToDateArr.every((isUpToDate) => isUpToDate);
+		} catch {
+			// If checking the up-to-date state fails, we assume that the build is not up to date.
+			return false;


We should log the exception and stack if possible.

maybe in verbose or debug trace?

Yeah, verbose.debug would be fine.

Note that the value of this is just to short circuit the build. So, it is ok to just return false, and let the task individually error out later.

tylerbutler · 2025-04-07T22:43:05Z

build-tools/packages/build-tools/src/fluidBuild/tscUtils.ts

+		tscUtilLibPathCache.set(tsPath, tscUtil);
+		return tscUtil;
+	} catch (e) {
+		throw new Error(`Failed to load typescript module for ${path}: ${e.message}`);


Include the stack.

Adding the inner stack here doesn't seem useful to me.... and it will cause it to have two set of stacks, may be we shouldn't add the try catch here to provide a friendlier error message?

changed to amending the existing error message and rethrow (thus keeping the original stack).

I see now. Sorry I misunderstood the flow here.

curtisman · 2025-04-07T22:46:22Z

In general, the pattern for the input/output might not be uniform for the whole repo. But it could be per package.

In my case though, it would have even nicer to allow regex on the command line to specify the input/output.

tylerbutler

Looks good to me. I'm interested to see this in action once you're using it.

tylerbutler · 2025-04-08T23:31:22Z

/azp run Build - api-markdown-documenter,Build - benchmark-tool,Build - build-common,Build - build-tools,Build - common-utils,Build - eslint-config-fluid,Build - eslint-plugin-fluid

tylerbutler · 2025-04-08T23:31:37Z

/azp run Build - protocol-definitions,Build - test-tools,server-gitrest,server-gitssh,server-historian,server-routerlicious,Build - client packages,repo-policy-check

azure-pipelines · 2025-04-08T23:31:38Z

Azure Pipelines successfully started running 1 pipeline(s).

azure-pipelines · 2025-04-08T23:31:49Z

Azure Pipelines successfully started running 1 pipeline(s).

curtisman · 2025-04-08T23:33:41Z

Looks good to me. I'm interested to see this in action once you're using it.

you can look at this: https://github.com/microsoft/TypeAgent/blob/297ff119c847b804825d5ce55a473163ee2b75db/ts/packages/agents/player/package.json#L45

curtisman added 2 commits March 19, 2025 16:03

Error

d0db370

declarative

f10217e

Copilot AI review requested due to automatic review settings March 19, 2025 23:13

github-actions bot added the area: build Build related issues label Mar 19, 2025

github-actions bot added the base: main PRs targeted against main branch label Mar 19, 2025

Copilot AI reviewed Mar 19, 2025

View reviewed changes

build-tools/packages/build-tools/src/fluidBuild/buildGraph.ts Show resolved Hide resolved

build-tools/packages/build-tools/src/fluidBuild/tscUtils.ts Outdated Show resolved Hide resolved

curtisman added 2 commits March 19, 2025 16:14

lint

58ad466

format

c97e882

curtisman force-pushed the declarative branch from ec45829 to c97e882 Compare March 19, 2025 23:29

curtisman added 2 commits March 19, 2025 16:30

message

a78eea6

lint

c16bf8f

tylerbutler self-assigned this Mar 31, 2025

tylerbutler requested a review from a team April 7, 2025 22:38

tylerbutler reviewed Apr 7, 2025

View reviewed changes

tylerbutler requested a review from a team April 7, 2025 22:44

fix lint and error stack

27302d9

tylerbutler approved these changes Apr 8, 2025

View reviewed changes

tylerbutler changed the title ~~Fluid-build: Per package Declarative Tasks~~ feat(fluid-build): Per package Declarative Tasks Apr 8, 2025

tylerbutler merged commit b5ce3f0 into microsoft:main Apr 8, 2025
38 checks passed

		node.pkg.packageJson.fluidBuild?.declarativeTasks?.[executable] ??
		declarativeTasks?.[executable];

feat(fluid-build): Per package Declarative Tasks #24100

feat(fluid-build): Per package Declarative Tasks #24100

Uh oh!

Conversation

curtisman commented Mar 19, 2025

Uh oh!

curtisman commented Mar 19, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Uh oh!

Uh oh!

tylerbutler commented Apr 7, 2025

Uh oh!

azure-pipelines bot commented Apr 7, 2025

Uh oh!

tylerbutler commented Apr 7, 2025

Uh oh!

azure-pipelines bot commented Apr 7, 2025

Uh oh!

tylerbutler commented Apr 7, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

curtisman Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

curtisman Apr 8, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

curtisman commented Apr 7, 2025

Uh oh!

tylerbutler left a comment

Choose a reason for hiding this comment

Uh oh!

tylerbutler commented Apr 8, 2025

Uh oh!

tylerbutler commented Apr 8, 2025

Uh oh!

azure-pipelines bot commented Apr 8, 2025

Uh oh!

azure-pipelines bot commented Apr 8, 2025

Uh oh!

curtisman commented Apr 8, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

curtisman Apr 8, 2025 •

edited

Loading

curtisman Apr 8, 2025 •

edited

Loading