Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use npm workspaces to reuse @sap/cds-mtxs from MTX Sidecar #228

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

beckermarc
Copy link
Contributor

npm workspaces can help to share @sap/cds-mtxs from MTX Sidecar with the version used by @sap/cds-dk and cds build.
Main downside at the moment: To retain fixed versions when using @sap/cds-dk this requires a package-lock.json in root.
In addition the package-lock.json in mtx/sidecar is required for cds build, as only this file is copied over to gen.

beckermarc and others added 6 commits May 5, 2023 14:31
Aligned MTA deployment files and MTX Sidecar with best practices
Introduced workspaces concept for mtx/sidecar to ensure availability of @sap/cds-mtxs in root node_modules
@beckermarc
Copy link
Contributor Author

Follow up of #224
FYI @LotharBender @swaldmann @mofterdinger

Base automatically changed from update-cdsdk-6.8.0 to main May 16, 2023 06:53
@LotharBender
Copy link
Contributor

I've just noticed that the cds-maven-plugin does not check whether the installed @sap/cds-dk version matches the configured version - the configured version is not installed.
In the node_modules folder @sap/[email protected] is installed. After updating <cds.install-cdsdk.version>6.8.0</cds.install-cdsdk.version> the command mvn clean install logs:

[INFO] InstallCdsdkMojo: Found locally installed @sap/cds-dk, skipping execution.

After deleting the node_modules folder and executing mvn clean install again, the @sap/cds-dk version is installed correctly:

[INFO] InstallCdsdkMojo: Executing [/Users/d022960/.m2/repository/com/sap/cds/cds-maven-plugin/cache/unpacked/16.19.0/unpacked-16.19.0-darwin-x64.tar.gz/node-v16.19.0-darwin-x64/bin/npm, install, @sap/[email protected], --no-save] in working directory /Users/d022960/git/work/java-mtx2

@mofterdinger
Copy link
Member

mofterdinger commented May 17, 2023

I've just noticed that the cds-maven-plugin does not check whether the installed @sap/cds-dk version matches the configured version - the configured version is not installed.

@LotharBender Hi Lothar, yes this behavior is described here: https://cap.cloud.sap/docs/java/assets/cds-maven-plugin-site/install-cdsdk-mojo.html:

By default, this goal looks for an already installed @sap/cds-dk and skips installation if it was found. It doesn't validate the found version against the requested version and the existing @sap/cds-dk could be outdated. Add property -Dcds.install-cdsdk.force=true to the Maven command line to force the installation of the @sap/cds-dk in the configured version.

@LotharBender
Copy link
Contributor

LotharBender commented May 17, 2023

By default, this goal looks for an already installed @sap/cds-dk and skips installation if it was found. It doesn't validate the found version against the requested version and the existing @sap/cds-dk could be outdated.

But doesn't this again prove that Java projects should better stick to the npm approach of maintaining and updating npm dependencies instead of using a maven based mechanism?

  • outdated local cds-dk versions have caused a lot of problems in the past. I have to admit that I haven't known the force update switch. I would vote for making this the default setting.
  • npm would correctly update the cds-dk version by default. A package-lock.json can be used to install fixed versions for @sap/mtxs and @sap/cds-dk.
  • a new version would only be downloaded if required. Does cds.install-force=true always download and install a new version, regardless whether the installed version already matches or not?

On the other hand this means that the default local development experience of the current cds-dk installation approach is not better than using npm workspaces without a package-lock.json in project root ;-)

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.

None yet

3 participants