Skip to content

Commit

Permalink
[FIX] Allow usage of after/before task assignment for all standard ta…
Browse files Browse the repository at this point in the history
…sks (#628)

This change allows custom tasks to subscribe to any standard task, even
disabled ones, ensuring custom tasks' execution and the correct order.

Background information:
When transitioning a project from V2 to V3, we often need to adjust the
before/after task assignments for custom tasks due to the removal of the
`uglify` task. The new `minify` task now runs earlier than the previous
`uglify` task.
Developers typically used `uglify` as the last enabled standard task to
subscribe to.

You can find updated documentation here:
SAP/ui5-tooling#874

JIRA: CPOUI5FOUNDATION-724

---------

Co-authored-by: Yavor Ivanov <[email protected]>
  • Loading branch information
flovogt and d3xter666 authored Sep 27, 2023
1 parent ec5d7cc commit 1a272d2
Show file tree
Hide file tree
Showing 8 changed files with 205 additions and 18 deletions.
44 changes: 26 additions & 18 deletions lib/build/TaskRunner.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,9 @@ class TaskRunner {
// This would require a more robust contract to identify task executions
// (e.g. via an 'id' that can be assigned to a specific execution in the configuration).
const taskWithoutSuffixCounter = taskName.replace(/--\d+$/, "");
return tasksToRun.includes(taskWithoutSuffixCounter);
return tasksToRun.includes(taskWithoutSuffixCounter) &&
// Task can be explicitly excluded by making its taskFunction = null
this._tasks[taskName].task !== null;
});

this._log.setTasks(allTasks);
Expand Down Expand Up @@ -176,25 +178,31 @@ class TaskRunner {
`It has already been scheduled for execution`);
}

const task = async (log) => {
options.projectName = this._project.getName();
options.projectNamespace = this._project.getNamespace();
let task;
if (taskFunction === null) {
this._log.verbose(`Task ${taskName} is set to be explicitly skipped in definitions.`);
task = null;
} else {
task = async (log) => {
options.projectName = this._project.getName();
options.projectNamespace = this._project.getNamespace();

const params = {
workspace: this._project.getWorkspace(),
taskUtil: this._taskUtil,
options
};

if (requiresDependencies) {
params.dependencies = this._allDependenciesReader;
}

const params = {
workspace: this._project.getWorkspace(),
taskUtil: this._taskUtil,
options
if (!taskFunction) {
taskFunction = (await this._taskRepository.getTask(taskName)).task;
}
return taskFunction(params);
};

if (requiresDependencies) {
params.dependencies = this._allDependenciesReader;
}

if (!taskFunction) {
taskFunction = (await this._taskRepository.getTask(taskName)).task;
}
return taskFunction(params);
};
}
this._tasks[taskName] = {
task,
requiredDependencies: requiresDependencies ? this._directDependencies : new Set()
Expand Down
5 changes: 5 additions & 0 deletions lib/build/definitions/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,11 @@ export default function({project, taskUtil, getTask}) {
}, Promise.resolve());
}
});
} else {
// No bundles defined. Just set task so that it can be referenced by custom tasks
tasks.set("generateBundle", {
taskFunction: null
});
}

tasks.set("generateVersionInfo", {
Expand Down
6 changes: 6 additions & 0 deletions lib/build/definitions/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,8 @@ export default function({project, taskUtil, getTask}) {
skipBundles: existingBundleDefinitionNames
}
});
} else {
tasks.set("generateComponentPreload", {taskFunction: null});
}

tasks.set("generateLibraryPreload", {
Expand Down Expand Up @@ -133,6 +135,8 @@ export default function({project, taskUtil, getTask}) {
}, Promise.resolve());
}
});
} else {
tasks.set("generateBundle", {taskFunction: null});
}

tasks.set("buildThemes", {
Expand All @@ -153,6 +157,8 @@ export default function({project, taskUtil, getTask}) {
version: project.getVersion()
}
});
} else {
tasks.set("generateThemeDesignerResources", {taskFunction: null});
}

tasks.set("generateResourcesJson", {
Expand Down
2 changes: 2 additions & 0 deletions lib/build/definitions/themeLibrary.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,8 @@ export default function({project, taskUtil, getTask}) {
version: project.getVersion()
}
});
} else {
tasks.set("generateThemeDesignerResources", {taskFunction: null});
}

tasks.set("generateResourcesJson", {requiresDependencies: true});
Expand Down
41 changes: 41 additions & 0 deletions test/lib/build/TaskRunner.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,6 +246,7 @@ test("_initTasks: Project of type 'library'", async (t) => {
"generateLibraryPreload",
"generateBundle",
"buildThemes",
"generateThemeDesignerResources",
"generateResourcesJson"
], "Correct standard tasks");
});
Expand Down Expand Up @@ -290,6 +291,7 @@ test("_initTasks: Project of type 'theme-library'", async (t) => {
"replaceCopyright",
"replaceVersion",
"buildThemes",
"generateThemeDesignerResources",
"generateResourcesJson"
], "Correct standard tasks");
});
Expand Down Expand Up @@ -1248,6 +1250,45 @@ test("Custom task: requiredDependenciesCallback returns Array instead of Set", a
}, "Threw with expected error message");
});

test("Custom task attached to a disabled task", async (t) => {
const {graph, taskUtil, taskRepository, TaskRunner, projectBuildLogger, sinon, customTask} = t.context;

const project = getMockProject("application");
const customTaskFnStub = sinon.stub();
project.getBundles = emptyarray;
project.getCustomTasks = () => [
{name: "myTask", afterTask: "generateBundle", configuration: "dog"}
];

taskRepository.getTask = sinon.stub().returns({task: sinon.stub()});
customTask.getTask = () => customTaskFnStub;

const taskRunner = new TaskRunner({
project, graph, taskUtil, taskRepository, log: projectBuildLogger, buildConfig
});

await taskRunner.runTasks();

const setTasksArgs = projectBuildLogger.setTasks.firstCall.args[0];
t.true(setTasksArgs.includes("myTask"), "Custom task 'myTask' is queried");
t.is(customTaskFnStub.calledOnce, true, "Custom task 'myTask' is executed");
t.false(setTasksArgs.includes("generateBundle"),
"generateBundle standard task is excluded from the execution list");

t.deepEqual(
setTasksArgs,
[
"escapeNonAsciiCharacters",
"replaceCopyright",
"replaceVersion",
"minify",
"generateFlexChangesBundle",
"generateComponentPreload",
"myTask",
],
"Correct tasks execution");
});

test.serial("_addTask", async (t) => {
const {sinon, graph, taskUtil, taskRepository, TaskRunner, projectBuildLogger} = t.context;

Expand Down
6 changes: 6 additions & 0 deletions test/lib/build/definitions/application.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,6 +83,9 @@ test("Standard build", (t) => {
requiresDependencies: true
},
transformBootstrapHtml: {},
generateBundle: {
taskFunction: null
},
generateVersionInfo: {
requiresDependencies: true,
options: {
Expand Down Expand Up @@ -159,6 +162,9 @@ test("Standard build with legacy spec version", (t) => {
requiresDependencies: true
},
transformBootstrapHtml: {},
generateBundle: {
taskFunction: null
},
generateVersionInfo: {
requiresDependencies: true,
options: {
Expand Down
110 changes: 110 additions & 0 deletions test/lib/build/definitions/library.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,15 @@ test("Standard build", async (t) => {
cssVariables: undefined
}
},
generateBundle: {
taskFunction: null
},
generateComponentPreload: {
taskFunction: null
},
generateThemeDesignerResources: {
taskFunction: null
},
generateResourcesJson: {
requiresDependencies: true
}
Expand Down Expand Up @@ -241,6 +250,15 @@ test("Standard build with legacy spec version", (t) => {
cssVariables: undefined
}
},
generateBundle: {
taskFunction: null
},
generateComponentPreload: {
taskFunction: null
},
generateThemeDesignerResources: {
taskFunction: null
},
generateResourcesJson: {
requiresDependencies: true
}
Expand Down Expand Up @@ -361,6 +379,12 @@ test("Custom bundles", async (t) => {
cssVariables: undefined
}
},
generateComponentPreload: {
taskFunction: null
},
generateThemeDesignerResources: {
taskFunction: null
},
generateResourcesJson: {
requiresDependencies: true
}
Expand Down Expand Up @@ -606,3 +630,89 @@ test("buildThemes: CSS Variables enabled", (t) => {
t.is(taskUtil.getBuildOption.getCall(0).args[0], "cssVariables",
"taskUtil#getBuildOption got called with correct argument");
});

test("Standard build: nulled taskFunction to skip tasks", (t) => {
const {project, taskUtil, getTask} = t.context;
project.getJsdocExcludes = () => ["**.html"];

const tasks = library({
project, taskUtil, getTask
});
const generateComponentPreloadTaskDefinition = tasks.get("generateComponentPreload");
const generateBundleTaskDefinition = tasks.get("generateBundle");
const generateThemeDesignerResourcesTaskDefinition = tasks.get("generateThemeDesignerResources");
t.deepEqual(Object.fromEntries(tasks), {
escapeNonAsciiCharacters: {
options: {
encoding: "UTF-412", pattern: "/**/*.properties"
}
},
replaceCopyright: {
options: {
copyright: "copyright",
pattern: "/**/*.{js,library,css,less,theme,html}"
}
},
replaceVersion: {
options: {
version: "version",
pattern: "/**/*.{js,json,library,css,less,theme,html}"
}
},
replaceBuildtime: {
options: {
pattern: "/resources/sap/ui/Global.js"
}
},
generateJsdoc: {
requiresDependencies: true,
taskFunction: async () => {},
},
executeJsdocSdkTransformation: {
requiresDependencies: true,
options: {
dotLibraryPattern: "/resources/**/*.library"
}
},
minify: {
options: {
pattern: [
"/resources/**/*.js",
"!**/*.support.js",
]
}
},
generateLibraryManifest: {},
generateLibraryPreload: {
options: {
excludes: [], skipBundles: []
}
},
buildThemes: {
requiresDependencies: true,
options: {
projectName: "project.b",
librariesPattern: undefined,
themesPattern: undefined,
inputPattern: "/resources/project/b/themes/*/library.source.less",
cssVariables: undefined
}
},
generateBundle: {
taskFunction: null
},
generateComponentPreload: {
taskFunction: null
},
generateThemeDesignerResources: {
taskFunction: null
},
generateResourcesJson: {
requiresDependencies: true
}
}, "Correct task definitions");

t.is(generateComponentPreloadTaskDefinition.taskFunction, null, "taskFunction is explicitly set to null");
t.is(generateBundleTaskDefinition.taskFunction, null, "taskFunction is explicitly set to null");
t.is(generateThemeDesignerResourcesTaskDefinition.taskFunction, null, "taskFunction is explicitly set to null");
});
9 changes: 9 additions & 0 deletions test/lib/build/definitions/themeLibrary.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ test("Standard build", (t) => {
const tasks = themeLibrary({
project, taskUtil, getTask
});
const generateThemeDesignerResourcesTaskFunction = tasks.get("generateThemeDesignerResources");
t.deepEqual(Object.fromEntries(tasks), {
replaceCopyright: {
options: {
Expand All @@ -72,12 +73,17 @@ test("Standard build", (t) => {
},
generateResourcesJson: {
requiresDependencies: true
},
generateThemeDesignerResources: {
taskFunction: null
}
}, "Correct task definitions");

t.is(taskUtil.getBuildOption.callCount, 1, "taskUtil#getBuildOption got called once");
t.is(taskUtil.getBuildOption.getCall(0).args[0], "cssVariables",
"taskUtil#getBuildOption got called with correct argument");

t.is(generateThemeDesignerResourcesTaskFunction.taskFunction, null, "taskFunction is explicitly set to null");
});

test("Standard build (framework project)", (t) => {
Expand Down Expand Up @@ -128,6 +134,9 @@ test("Standard build for non root project", (t) => {
},
generateResourcesJson: {
requiresDependencies: true
},
generateThemeDesignerResources: {
taskFunction: null
}
}, "Correct task definitions");

Expand Down

0 comments on commit 1a272d2

Please sign in to comment.