[BUG] @cap-js/cds-types v0.6.0 - Post install script not working #136

geert-janklaps · 2024-07-14T07:57:51Z

Is there an existing issue for this?

I have searched the existing issues

Current Behavior

Currently when enabling @sap/cds with typescript (using cds --add typescript, typer) and after reinstalling the dependencies, the post installation script doesn't create the symbolic link to the @types folder.

This results in not being able to use the following statement in ts files: import cds from '@sap/cds'.

This can be fixed by manually creating the symbolic link as follows (on Linux):
ln -s /workspaces/advanced-output-management/node_modules/@cap-js/cds-types node_modules/@types/sap__cds

I'm running the project in a dev container with image: mcr.microsoft.com/devcontainers/typescript-node:1-20-bookworm

Expected Behavior

The post install script to automatically create the necessary symbolic link

References

not applicable

Versions

advanced-output-manage
@cap-js/asyncapi	1.0.1
@cap-js/cds-typer	0.23.0
@cap-js/cds-types	0.6.0
@cap-js/hana	1.1.0
@cap-js/openapi	1.0.4
@cap-js/sqlite	1.7.3
@sap/cds	8.0.3
@sap/cds-compiler	5.0.6
@sap/cds-dk	8.0.2
@sap/cds-dk (global)	8.0.2
@sap/cds-fiori	1.2.7
@sap/cds-foss	5.0.1
@sap/cds-mtxs	2.0.2
@sap/eslint-plugin-cds	3.0.4
Node.js	v20.14.0
home	/workspaces/advanced-output-management/node_modules/@sap/cds

Anything else? Logs?

Devcontainer.json configuration:

// For format details, see https://aka.ms/devcontainer.json. For config options, see the
// README at: https://github.com/devcontainers/templates/tree/main/src/typescript-node
{
	"name": "advanced-output-management-devcontainer",
	// Or use a Dockerfile or Docker Compose file. More info: https://containers.dev/guide/dockerfile
	"image": "mcr.microsoft.com/devcontainers/typescript-node:1-20-bookworm",
	// Features to add to the dev container. More info: https://containers.dev/features.
	// "features": {},
	// Use 'forwardPorts' to make a list of ports inside the container available locally.
	"forwardPorts": [
		4004,
		5000
	],

	// Uncomment to connect as root instead. More info: https://aka.ms/dev-containers-non-root.
	"remoteUser": "root"
}

The text was updated successfully, but these errors were encountered:

dinurp · 2024-07-17T07:56:07Z

This issue is causing errors in initializing a sample cap project.

PS D:\temp> cds init cap-app-sample
Creating new CAP project in .\cap-app-sample

Adding feature 'nodejs'...

Successfully created project. Continue with 'cd cap-app-sample'.
Find samples on https://github.com/SAP-samples/cloud-cap-samples
Learn about next steps at https://cap.cloud.sap
PS D:\temp> cd .\cap-app-sample\
PS D:\temp\cap-app-sample> cds add sample
Adding feature 'sample'...

Successfully added features to your project.
PS D:\temp\cap-app-sample> cds -v -i

| cap-app-sample         | <Add your repository here>                                                        |
| ---------------------- | --------------------------------------------------------------------------------- |
| @cap-js/asyncapi       | 1.0.1                                                                             |
| @cap-js/openapi        | 1.0.4                                                                             |
| @sap/cds               | 8.0.3                                                                             |
| @sap/cds-compiler      | 5.0.6                                                                             |
| @sap/cds-dk (global)   | 8.0.2                                                                             |
| @sap/cds-fiori         | 1.2.7                                                                             |
| @sap/cds-foss          | 5.0.1                                                                             |
| @sap/cds-mtxs          | 2.0.2                                                                             |
| @sap/eslint-plugin-cds | 3.0.4                                                                             |
| Node.js                | v18.16.0                                                                          |
| home                   | C:\Users\me\AppData\Roaming\npm\node_modules\@sap\cds-dk\node_modules\@sap\cds |

PS D:\temp\cap-app-sample> npm i
npm ERR! code 1
npm ERR! path D:\temp\cap-app-sample\node_modules\@cap-js\cds-types
npm ERR! command failed
npm ERR! command C:\Windows\system32\cmd.exe /d /s /c ./scripts/postinstall.js
npm ERR! '.' is not recognized as an internal or external command,
npm ERR! operable program or batch file.

npm ERR! A complete log of this run can be found in:
npm ERR!     C:\Users\me\AppData\Local\npm-cache\_logs\2024-07-17T07_23_13_223Z-debug-0.log

package.json has the following entry

  "devDependencies": {
    "@cap-js/sqlite": "^1",
    "@cap-js/cds-types": "^0.6"
  },

changing this to the following allows to proceed with initializing sample project.

  "devDependencies": {
    "@cap-js/sqlite": "^1",
    "@cap-js/cds-types": "^0.2"
  },

daogrady · 2024-07-17T07:59:25Z

Hi everyone,

thanks for pointing out this problem on Windows. We will see what we can do to unblock Windows users.

Best,
Daniel

geert-janklaps · 2024-07-17T08:15:46Z

Hi @daogrady,

I'm not sure if you are aware, but this issue can also be solved without creating a symbolic link.
I've adapted my tsconfig.json with typeRoots like so:

{
  "compilerOptions": {
    "target": "ESNext",
    "module": "NodeNext",
    "moduleResolution": "NodeNext",
    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true,
    "strict": true,
    "skipLibCheck": true,
    "sourceMap": true,
    "allowJs": true,
    "paths": {
      "#cds-models/*": [
        "./@cds-models/*/index.ts"
      ]
    },
    "typeRoots": [
      "./node_modules/@types",
      "./node_modules/@cap-js"
    ]
  }
}

This is working perfectly fine without the need of a symbolic link. (similar setup like the UI5 team does for the UI5 types)
With that minor difference, that in this case we can't specify the @cap-js/cds-types package explicitly (probably because of the way the package structure is setup, in this case the types are in a dist folder while in the UI5 types these are in a types folder)

Anyway, this feels like a more solid and standard TypeScript configuration. Might be worth considering for you guys :)
(And in the meanwhile, takes away the complexity of handling different OS's)

Cheers,
Geert-Jan

daogrady · 2024-07-17T08:21:11Z

Hi Geert-Jan,

thanks for pointing this out! We have considered an additional typeRoots entry (among a few other options), but wanted to not burden the user with any additional config they would have to add to their project (for JS projects, this could even mean the user would need an entire jsconfig.json for just this entry).

If having the typeRoots entry works for you then that is perfectly fine for your project to use! 👍
Still, I hope to be able to come up with a solution that works for everyone out of the box.

Best,
Daniel

daogrady · 2024-07-18T06:19:18Z

Hi everyone,

we just released cds-types 0.6.1 which contains a fix for the post install script on windows which should address this issue.
Should the problem persist, feel free to reopen this issue or open a follow-up.

Best,
Daniel

geert-janklaps · 2024-07-18T06:59:37Z

Hi @daogrady,

I opened the issue on Linux, not on Windows.
I've just checked, v0.6.1 still causes issues on Linux.
I get the same issue on my local installation and in a container running as a GitHub action:

Cheers,
Geert-Jan

daogrady · 2024-07-18T07:25:49Z

Hi Geert-Jan,

sorry for the inconvenience! Is this the same error you encountered originally? It looks like this error pops up in the context of building/ bundling?

Best,
Daniel

geert-janklaps · 2024-07-18T07:44:27Z

Hi @daogrady,

Seems like a slightly different situation indeed.
What seems to be the issue I'm encountering:

Existing project running @sap/cds @ 7.9.3
No explicit installation of @cap-js/types
Running npm install -> works fine (adds dependency to @cap-js/cds-types @ 0.2.0)
Running npm update --package-lock-only -> causes error (adds dependency to @cap-js/cds-types @ 0.6.1)

Based on the configuration the last situation shouldn't happen (as @cap-js/cds-types has peer dependency to @sap/cds @ 8)

I'm able to fix this by explicitly installing @cap-js/cds-types @0.2.0 in the project.

I also retested the initial issue (on cds v8) and this issue does seem to be resolved. So maybe lets close this issue, I'll do some further analysis on the existing project and if needed I'll open a new issue.

Cheers,
Geert-Jan

daogrady · 2024-07-18T07:53:14Z

Hi Geert-Jan,

thanks for the analysis of the problem! That is indeed an issue, as cds-types 0.6 should only be used in combination with cds 8. Pipelines pulling in cds-types 0.6 despite its peerDependency to cds 8 is not the desired behaviour and will lead to problems. I will investigate this.

Best,
Daniel

daogrady · 2024-07-18T07:53:50Z

Hi @daogrady,

Seems like a slightly different situation indeed. What seems to be the issue I'm encountering:

Existing project running @sap/cds @ 7.9.3

No explicit installation of @cap-js/types

Running npm install -> works fine (adds dependency to @cap-js/cds-types @ 0.2.0)

Running npm update --package-lock-only -> causes error (adds dependency to @cap-js/cds-types @ 0.6.1)

Based on the configuration the last situation shouldn't happen (as @cap-js/cds-types has peer dependency to @sap/cds @ 8)

I'm able to fix this by explicitly installing @cap-js/cds-types @0.2.0 in the project.

I also retested the initial issue (on cds v8) and this issue does seem to be resolved. So maybe lets close this issue, I'll do some further analysis on the existing project and if needed I'll open a new issue.

Cheers, Geert-Jan

@chgeo FYI

daogrady · 2024-07-18T07:55:26Z

related: #136

daogrady · 2024-07-18T08:04:08Z

@geert-janklaps @hakimio

thanks for pointing out this incompatibility with cds7 projects. The problem stems from an open dependency range in cds7 which we will fix asap.

chgeo · 2024-07-18T08:07:16Z

@daogrady that's seems to be the case I was worried about yesterday in out discussion.
Yes, it's probably worth closing the dependency range in the next sap/cds patch 7.9.x

dinurp · 2024-07-18T08:25:35Z

Thanks for the fix. Initializing a sample project with CDS@8 works fine now with this this fix.

PS D:\temp\cap-app-sample> npm ls
cap-app-sample@1.0.0 D:\temp\cap-app-sample
+-- @cap-js/cds-types@0.6.1
+-- @cap-js/sqlite@1.7.3
+-- @sap/cds@8.0.3
`-- express@4.19.2

devinea · 2024-07-18T12:00:54Z

@daogrady, @chgeo
I think part of the problem with CDS V7 projects is that the postinstall script assumes the folder location for the symlink target. https://github.com/cap-js/cds-types/blob/main/scripts/postinstall.js#L17
Would be be more robust to use const pathToModule = require.resolve('@cap-js/cds-types'); so the symlink always has a valid target

chgeo · 2024-07-18T12:11:44Z

Could point. Should check if this is more robust.

daogrady · 2024-07-22T11:05:46Z

Hi @devinea,

thanks for the suggestion! While I see the general appeal of the more generic approach, I have a hard time seeing the case in this specific scenario. As the script is executed as part of the installation of @cap-js/cds-types, it should always find that package. Could you please point out the circumstances under which targeting the specific folder location is incorrect?

Best,
Daniel

daogrady · 2024-08-05T05:58:37Z

Hi everyone,

has been rather quiet around the install script lately, so I assume this has been fixed for now. Therefore closing the issue. 🙂

Best,
Daniel

geert-janklaps added the bug Something isn't working label Jul 14, 2024

daogrady closed this as completed Jul 18, 2024

daogrady reopened this Jul 18, 2024

daogrady closed this as completed Aug 5, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[BUG] @cap-js/cds-types v0.6.0 - Post install script not working #136

[BUG] @cap-js/cds-types v0.6.0 - Post install script not working #136

geert-janklaps commented Jul 14, 2024 •

edited

Loading

dinurp commented Jul 17, 2024

daogrady commented Jul 17, 2024

geert-janklaps commented Jul 17, 2024 •

edited

Loading

daogrady commented Jul 17, 2024 •

edited

Loading

daogrady commented Jul 18, 2024 •

edited

Loading

geert-janklaps commented Jul 18, 2024

daogrady commented Jul 18, 2024 •

edited

Loading

geert-janklaps commented Jul 18, 2024

daogrady commented Jul 18, 2024

daogrady commented Jul 18, 2024

daogrady commented Jul 18, 2024

daogrady commented Jul 18, 2024

chgeo commented Jul 18, 2024

dinurp commented Jul 18, 2024

devinea commented Jul 18, 2024

chgeo commented Jul 18, 2024

daogrady commented Jul 22, 2024

daogrady commented Aug 5, 2024

[BUG] @cap-js/cds-types v0.6.0 - Post install script not working #136

[BUG] @cap-js/cds-types v0.6.0 - Post install script not working #136

Comments

geert-janklaps commented Jul 14, 2024 • edited Loading

Is there an existing issue for this?

Current Behavior

Expected Behavior

References

Versions

Anything else? Logs?

dinurp commented Jul 17, 2024

daogrady commented Jul 17, 2024

geert-janklaps commented Jul 17, 2024 • edited Loading

daogrady commented Jul 17, 2024 • edited Loading

daogrady commented Jul 18, 2024 • edited Loading

geert-janklaps commented Jul 18, 2024

daogrady commented Jul 18, 2024 • edited Loading

geert-janklaps commented Jul 18, 2024

daogrady commented Jul 18, 2024

daogrady commented Jul 18, 2024

daogrady commented Jul 18, 2024

daogrady commented Jul 18, 2024

chgeo commented Jul 18, 2024

dinurp commented Jul 18, 2024

devinea commented Jul 18, 2024

chgeo commented Jul 18, 2024

daogrady commented Jul 22, 2024

daogrady commented Aug 5, 2024

geert-janklaps commented Jul 14, 2024 •

edited

Loading

geert-janklaps commented Jul 17, 2024 •

edited

Loading

daogrady commented Jul 17, 2024 •

edited

Loading

daogrady commented Jul 18, 2024 •

edited

Loading

daogrady commented Jul 18, 2024 •

edited

Loading