[FEATURE] Add 'UI5 Workspace' Support #494

RandomByte · 2022-11-04T15:22:44Z

CPOUI5FOUNDATION-582

coveralls · 2022-11-16T20:40:17Z

Coverage: 95.41% (+0.6%) from 94.86% when pulling a807d70 on local-dependency-resolution-poc into f90d17a on main.

Implementing SAP/ui5-tooling#157

Pass it to NodePackageDependencies-Provider and ui5FrameworkHelper

…provided

…a workspaces

…of module ID This should make the use of workspaces easier. Framework projects are matched based on the name in the ui5.yaml instead of the package.json name (= module ID). This allows for example framework projects to be contained in packages that are named differently from the local copy. E.g. "@openui5/sap.m-prebuilt" while the local copy does not have the "-prebuilt" suffix. Note however, that the NodePackageDependencies-provider still matches nodes using the module ID. The basic reason for this difference is that framework dependencies are typically declared using the project name, while npm dependencies are declared using the module ID. However, we could also enhance projectGraphBuilder in the future to additionally check whether a project can be resolved with a workspace once the project has been created and its name is known. This could catch those cases where the node provider could not find a workspace resolution for a given module ID.

… via workspace

This means resolution paths will always be resolved relative to the workspace config file location instead of the process CWD. Refactored some graph code to make creating workspaces reusable for other graph factory functions in the future. Extended related tests.

If no project is found, return null instead of undefined

While we don't expect this to happen in released UI5 versions, it might happen with development setups using UI5 Workspaces

flovogt

Awesome work @RandomByte. Well done!

lib/graph/ProjectGraph.js

lib/graph/Workspace.js

flovogt · 2023-01-31T14:32:28Z

lib/graph/Workspace.js

+
+	/**
+	 * @param {object} options
+	 * @param {object} options.cwd Path to use for resolving all paths of the workspace configuration from.


cwd is of type string, isn't?

test/lib/graph/graph.js

test/lib/graph/helpers/createWorkspace.js

flovogt · 2023-01-31T15:37:04Z

test/lib/graph/helpers/createWorkspace.js

+});
+
+test.afterEach.always((t) => {
+	t.context.sinon.restore();


missing esmock.purge(t.context.createWorkspace)

I read https://github.com/iambumblehead/esmock/wiki#calling-esmock-await-import and came to think that this might not be necessary in this case.

esmock creates a new mock for every test in parallel. Cleaning up after each test to free memory (or for any other reason) seems unnecessary since ava executes this file in a separate process which it will kill after all tests are done.

I remember we had to do it when await import is used.... Interesting

Yes, because await import requires the delayed esmock.p API which might interfere with other tests and therefore would require all tests to use .serial().

As it turns out, we don't use await import for "Workspace.js" anymore (this was used in an earlier version of this PR) and esmock is enough 👍

test/lib/graph/helpers/createWorkspace.js

Co-authored-by: Florian Vogt <florian.vogt@sap.com>

Keep modules that only contain extensions instead of discarding them. This allows to use workspaces for resolving extensions too. Before, only extensions in modules that also contained projects where resolved.

RandomByte

Thank you very much @flovogt 😊

lib/graph/ProjectGraph.js

lib/graph/Workspace.js

RandomByte · 2023-02-01T13:17:01Z

lib/graph/Workspace.js

+
+	/**
+	 * @param {object} options
+	 * @param {object} options.cwd Path to use for resolving all paths of the workspace configuration from.


RandomByte · 2023-02-01T13:59:50Z

lib/graph/helpers/ui5Framework.js

+						`Try adding it temporarily to the root project's dependencies`);
+				}
+
+				// TODO: If a cyclic dependency is declared, this will empty the event loop.


This comment is obsolete. I solved this problem by taking in the _checkCycle function from the ProjectGraph. I dreamed that I forgot to remove this comment but the next morning I thought it was just a dream. Oh well...

lib/graph/helpers/ui5Framework.js

RandomByte · 2023-02-01T14:00:12Z

test/fixtures/library.d-adtl-deps/main/src/library/d/some.js

+/*!
+ * ${copyright}
+ */
+console.log('HelloWorld');


RandomByte · 2023-02-01T14:00:16Z

test/fixtures/extension.a/ui5.yaml

+metadata:
+  name: extension.a-ui5-yaml
+task:
+  path: lib/extensionModule.js


test/lib/graph/graph.js

lib/graph/Workspace.js

flovogt · 2023-02-01T14:26:31Z

lib/graph/Workspace.js

+
+	/**
+	 * For a given project name (e.g. the value of the <code>metadata.name</code> property in a ui5.yaml),
+	 * returns a Module instance or undefined depending on whether the project


Suggested change

* returns a Module instance or undefined depending on whether the project

* returns a module instance or <code>undefined</code> depending on whether the project

flovogt · 2023-02-01T14:27:15Z

lib/graph/Workspace.js

+	 *
+	 * @public
+	 * @param {string} projectName Name of the project
+	 * @returns {Promise<@ui5/project/graph/Module|undefined>} Module instance of undefined if none is found


Suggested change

* @returns {Promise<@ui5/project/graph/Module|undefined>} Module instance of undefined if none is found

* @returns {Promise<@ui5/project/graph/Module|undefined>} Module instance of <code>undefined</code> if none is found

flovogt · 2023-02-01T14:42:25Z

lib/graph/Workspace.js

+
+	/**
+	 * For a given node id (e.g. the value of the name property in a package.json),
+	 * returns a Module instance or undefined depending on whether the module


Suggested change

* returns a Module instance or undefined depending on whether the module

* returns a module instance or <code>undefined</code> depending on whether the module

flovogt · 2023-02-01T14:43:11Z

lib/graph/Workspace.js

+	 *
+	 * @public
+	 * @param {string} nodeId Node ID of the module
+	 * @returns {Promise<@ui5/project/graph/Module|undefined>} Module instance of undefined if none is found


Suggested change

* @returns {Promise<@ui5/project/graph/Module|undefined>} Module instance of undefined if none is found

* @returns {Promise<@ui5/project/graph/Module|undefined>} Module instance of <code>undefined</code> if none is found

lib/graph/providers/NodePackageDependencies.js

test/fixtures/collection.b/library.b/package.json

test/lib/graph/graph.js

flovogt · 2023-02-01T14:50:07Z

test/lib/graph/helpers/createWorkspace.js

+});
+
+test.afterEach.always((t) => {
+	t.context.sinon.restore();


I remember we had to do it when await import is used.... Interesting

test/lib/graph/helpers/createWorkspace.js

matz3 · 2023-02-06T13:08:17Z

lib/graph/Workspace.js

+ * @public
+ * @typedef {object} @ui5/project/graph/Workspace~Configuration
+ * @property {string} node.specVersion
+ * @property {object} node.metadata Version of the project


Should be something like "Metadata of the configuration`, right?

Thanks! Correction in #568

Co-authored-by: Florian Vogt <florian.vogt@sap.com>

RandomByte force-pushed the local-dependency-resolution-poc branch from 615a153 to 1504c7a Compare November 4, 2022 16:01

RandomByte changed the base branch from main to snapshot-poc November 4, 2022 17:55

RandomByte force-pushed the snapshot-poc branch from 198aa78 to 21f5b0c Compare November 9, 2022 12:37

RandomByte force-pushed the local-dependency-resolution-poc branch 2 times, most recently from a8d63dd to 6f14964 Compare November 9, 2022 14:41

RandomByte changed the base branch from snapshot-poc to main November 9, 2022 14:41

RandomByte changed the title ~~[FEATURE][PoC] Enable snapshot consumption from Maven repository~~ [FEATURE][PoC] Add 'UI5 Workspace' Support Nov 9, 2022

RandomByte force-pushed the local-dependency-resolution-poc branch from 6f14964 to b811811 Compare November 15, 2022 23:14

RandomByte force-pushed the local-dependency-resolution-poc branch 2 times, most recently from a566ebd to 73f016f Compare November 17, 2022 00:01

RandomByte force-pushed the local-dependency-resolution-poc branch from 73f016f to 7847b89 Compare December 8, 2022 19:45

RandomByte force-pushed the local-dependency-resolution-poc branch from 7847b89 to 0ba89b7 Compare January 18, 2023 18:15

RandomByte mentioned this pull request Jan 18, 2023

[INTERNAL] ui5-workspace.yaml schema #543

Merged

RandomByte force-pushed the local-dependency-resolution-poc branch 4 times, most recently from 6663b14 to a8d996f Compare January 25, 2023 15:46

RandomByte changed the title ~~[FEATURE][PoC] Add 'UI5 Workspace' Support~~ [FEATURE] Add 'UI5 Workspace' Support Jan 27, 2023

RandomByte mentioned this pull request Jan 30, 2023

[INTERNAL] Add workspace arguments SAP/ui5-cli#602

Merged

RandomByte added 10 commits January 30, 2023 17:16

[FEATURE] Add 'UI5 Workspace' Support

0f02ea0

Implementing SAP/ui5-tooling#157

[INTERNAL] graphFromPackageDependencies: Create 'UI5 Workspace'

40764f4

Pass it to NodePackageDependencies-Provider and ui5FrameworkHelper

[INTERNAL] NodePackageDependencies: Resolve using 'UI5 Workspace' if …

9149d75

…provided

[INTERNAL] ui5Framework: Use Workspace to override paths of single nodes

9859c43

[INTERNAL] ui5Framework: Resolve dependencies of projects resolved vi…

941a010

…a workspaces

[INTERNAL] Workspace: Resolve package.json workspaces

65fee12

[INTERNAL] NodePackageDependencies: Add info log when resolving nodes…

cdffdea

… via workspace

[INTERNAL] Workspace: Fix package workspace resolution, update tests

4bdd7cd

[INTERNAL] Workspace: Enhance test coverage

26f2350

RandomByte added 11 commits January 30, 2023 17:20

[INTERNAL] helpers/ui5Framework: Enhance test coverage

41c2e5f

[INTERNAL] Workspace: Adapt to new @ui5/logger API

717309e

[INTERNAL] graph#utils: Add workspace schema validation

065b61a

[INTERNAL] Module: Add test for empty and corrupt YAML files

4b5ce09

If no project is found, return null instead of undefined

[INTERNAL] Workspace: Resolve nested workspaces

0165e46

[INTERNAL] graph: Fix tests on Windows

34e102c

[INTERNAL] graph: Refactor Workspace creation into helper module

d003244

[INTERNAL] ui5Framework: Add detection for cyclic dependencies

caf2bc5

While we don't expect this to happen in released UI5 versions, it might happen with development setups using UI5 Workspaces

[INTERNAL] ProjectGraph: Improve comments, error handling, tests

649107e

[INTERNAL] Workspace: Log resolved versions

8a5c1b8

RandomByte force-pushed the local-dependency-resolution-poc branch from feb2b82 to 8a5c1b8 Compare January 30, 2023 16:25

RandomByte marked this pull request as ready for review January 30, 2023 16:30

RandomByte requested a review from a team January 30, 2023 16:30

flovogt requested changes Jan 31, 2023

View reviewed changes

RandomByte and others added 4 commits February 1, 2023 14:54

[INTERNAL] Apply suggestions from code review

e006d52

Co-authored-by: Florian Vogt <florian.vogt@sap.com>

[INTERNAL] Workspace: Also use modules containing extensions only

c326827

Keep modules that only contain extensions instead of discarding them. This allows to use workspaces for resolving extensions too. Before, only extensions in modules that also contained projects where resolved.

[INTERNAL] Workspace: Add JSDoc and validation

dd7281a

[INTERNAL] Workspace: Cleanup based on code review

f16d3f2

RandomByte commented Feb 1, 2023

View reviewed changes

flovogt reviewed Feb 1, 2023

View reviewed changes

[INTERNAL] Workspace: Apply suggestions from code review

a807d70

flovogt approved these changes Feb 2, 2023

View reviewed changes

RandomByte merged commit b77ca2f into main Feb 2, 2023

RandomByte deleted the local-dependency-resolution-poc branch February 2, 2023 09:43

RandomByte mentioned this pull request Feb 3, 2023

[RFC] 0006 Local Dependency Resolution SAP/ui5-tooling#157

Merged

matz3 reviewed Feb 6, 2023

View reviewed changes

d3xter666 pushed a commit that referenced this pull request Feb 7, 2023

[FEATURE] Add 'UI5 Workspace' Support (#494)

08d69e7

Co-authored-by: Florian Vogt <florian.vogt@sap.com>

flovogt mentioned this pull request Feb 10, 2023

[ROADMAP] Specification Version 3.0 // UI5 Tooling 3.0 SAP/ui5-tooling#506

Closed

17 tasks

d3xter666 mentioned this pull request Feb 22, 2023

[FEATURE] Enable snapshot consumption from Maven repository #570

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[FEATURE] Add 'UI5 Workspace' Support #494

[FEATURE] Add 'UI5 Workspace' Support #494

RandomByte commented Nov 4, 2022 •

edited

coveralls commented Nov 16, 2022 •

edited

flovogt left a comment

flovogt Jan 31, 2023

RandomByte Feb 1, 2023

flovogt Jan 31, 2023

RandomByte Feb 1, 2023

flovogt Feb 1, 2023

RandomByte Feb 1, 2023

RandomByte left a comment

RandomByte Feb 1, 2023

RandomByte Feb 1, 2023

RandomByte Feb 1, 2023

RandomByte Feb 1, 2023

flovogt Feb 1, 2023

RandomByte Feb 1, 2023

flovogt Feb 1, 2023

RandomByte Feb 1, 2023

flovogt Feb 1, 2023

RandomByte Feb 1, 2023

flovogt Feb 1, 2023

RandomByte Feb 1, 2023

flovogt Feb 1, 2023

matz3 Feb 6, 2023

RandomByte Feb 6, 2023

	* returns a Module instance or undefined depending on whether the project
	* returns a module instance or <code>undefined</code> depending on whether the project

	* @returns {Promise<@ui5/project/graph/Module\|undefined>} Module instance of undefined if none is found
	* @returns {Promise<@ui5/project/graph/Module\|undefined>} Module instance of <code>undefined</code> if none is found

[FEATURE] Add 'UI5 Workspace' Support #494

[FEATURE] Add 'UI5 Workspace' Support #494

Conversation

RandomByte commented Nov 4, 2022 • edited

coveralls commented Nov 16, 2022 • edited

flovogt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RandomByte left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

RandomByte commented Nov 4, 2022 •

edited

coveralls commented Nov 16, 2022 •

edited