ui5-community / generator-ui5-ts-app-fcl

UI5 TypeScript application using the FlexibleColumnLayout. This generator was build as a plug-in for the community project Easy-UI5 by SAP.
Apache License 2.0
5 stars 4 forks source link

TS2688 Cannot find type definition file for 'node_modules'. #5

Closed Fjaoos closed 2 years ago

Fjaoos commented 2 years ago

Hi,

I used the yo generator to create a TS app but it shows the following error when you execute tsc --noEmit:

error TS2688: Cannot find type definition file for 'node_modules'. The file is in the program because: Entry point for implicit type library 'node_modules'

The app has been generated with default values only.

I did some research but could not come up with any solution.

lemaiwo commented 2 years ago

@petermuessig , do you have any idea?

akudev commented 2 years ago

@Fjaoos Can you please try the following change? In tsconfig.json replace:

        "typeRoots": [
            "./node_modules/@types",
            "./node_modules/@openui5/ts-types-esm"
        ],

with

        "typeRoots": [
            "./node_modules/@types"
        ],
            "types": [
            "@openui5/ts-types-esm"
        ],

(or replace "openui5" with "sapui5", if you used SAPUI5 in the generator)

wridgeu commented 2 years ago

Hey @akudev

I've tried it locally as well (did some thinking yday as well) the change you described works for me. I've checked several issues, the ts docs regarding typeroots/types, blogs and what not, the most prominent probably being https://github.com/microsoft/TypeScript/issues/27956.

If it isn't too much or too long to explain, may I ask why it is that this config change is working? I'm trying to make some sense of it for the future. ^^ Appreciate it!

Before image

After image

Fjaoos commented 2 years ago

Hi @akudev, that fix works very well. Might be a chapter for the FAQ. Thank you for your fix but I would also like to know why that works how it does.

Regards fjaoos

akudev commented 2 years ago

@Fjaoos , @wridgeu I'll let you know and document it once I figured out why it works. ;-) The suggestion came up during the UI5con workshop, so first prio was to apply the change everywhere and inform people how to do it. Second prio (after other things) will be to understand the behavior in detail...

akudev commented 2 years ago

@wridgeu @Fjaoos @lemaiwo Ok, it's now understood - albeit a bit lengthy to explain, as several tricky details come together:

A few months back (in 1.100) the UI5 types started coming with a package.json dependency to the jQuery and qUnit types. The goal was to a) free app developers from explicitly requiring them and to b) ensure the version of those types is included which best fits the jQuery/qUnit version actually embedded in UI5. As result, the jQuery types also end up in <appfolder>/node_modules when @openui5/ts-types-esm is added as dependency and are picked up by TypeScript. (of course all the same for @sapui5)

Because the @openui5/ts-types-esm types are not in the default @types package, one has to mention them explicitly in the tsconfig.json. We decided for using the "typeRoots" property for that because when using "types" ONLY these types would be available (at least globally) and one would have to explicitly list all types needed for global usage. The second typeRoots entry was for the standard folder:

        "typeRoots": [
            "./node_modules/@openui5/ts-types-esm",
            "./node_modules/@types"
        ],

This was the recommendation since 1.100.

But this fcl app template and also the original TypeScript Easy-UI5 template used for the UI5con workshop, where the same issue was discussed, had for some reason (probably because it was formerly required) its own dependency to the jQuery types. And still has. To a different version of the jQuery types than the one UI5 has as dependency! How does npm solve this conflict? The app-required jQuery types go right to <appfolder>/node_modules, the ones required by the UI5 types go to <appfolder>/node_modules/@openui5/ts-types-esm/node_modules! So suddenly there is a new folder below ts-types-esm which did not exist before.

The official documentation for "typeRoots" does not mention it at all, but an old comment by the TypeScript dev team lead made me understand that TypeScript expects ALL folders which are direct children of a "typeRoot" to be "types folders" and that each such "types folder" MUST have an index.d.ts file or a package.json. The new nested node_modules folder, however, has neither as direct content (even though two levels below there is such a types folder for the jQuery types), hence the error message "TS2688 Cannot find type definition file for 'node_modules'" which this issue report is about.

It basically complains that the node_modules folder within @openui5/ts-types-esm does not contain types although as child of a "typesRoot" folder it should.

The solution proposed above works because the OpenUI5 types are no longer in the "typesRoot" section, but in the "types" section. TypeScript hence just loads the types and is happy, ignoring the node_modules folder below.

        "typeRoots": [
            "./node_modules/@types"
        ],
        "types": [
            "@openui5/ts-types-esm"
        ],

However, there is one drawback: as mentioned before, making @openui5/ts-types-esm a "types" entry causes any other types not to be available globally. They ARE still available when the respective module is imported and also available for Quick Fixes, so it is not that dramatic. But you e.g. cannot use the global jQuery object anymore without importing it as module. (well, you can use it at runtime, but TypeScript won't know it) The question is whether one should do so, but anyway, the solution is simple: just add it to the "types" section.

        "typeRoots": [
            "./node_modules/@types"
        ],
        "types": [
            "@openui5/ts-types-esm",
            "jQuery"
        ],

And indeed right now it LOOKS LIKE the "typeRoots" entry for the standard types might not be needed at all, so the following might also suffice as solution:

        "types": [
            "@openui5/ts-types-esm",
            "jQuery"
        ],

On the other hand we know now what the error message was about and how to avoid it (only require the jQuery types ONCE). When doing so, the original way of pointing to the types continues to work fine:

        "typeRoots": [
            "./node_modules/@types",
            "./node_modules/@openui5/ts-types-esm"
        ],

One thing to keep in mind: as soon as the jQuery types have gone to the nested node_modules folder, they won't move up to the root-level one, even when the app dependency is removed. You have to delete them (possibly also package-lock.json) and run npm i again.

We haven't made up our mind yet which way to recommend and not fully tested all implications, but at least it is now pretty clear what happens and what the available options are.

wridgeu commented 2 years ago

wow @akudev. Thank you so so so much for the detailed explanation. Much appreciated!

The official documentation for "typeRoots" does not mention it at all, but https://github.com/microsoft/TypeScript/issues/35440#issuecomment-568123091 made me understand that TypeScript expects ALL folders which are direct children of a "typeRoot" to be "types folders" and that each such "types folder" MUST have an index.d.ts file or a package.json. The new nested node_modules folder, however, has neither as direct content (even though two levels below there is such a types folder for the jQuery types), hence the error message "TS2688 Cannot find type definition file for 'node_modules'" which this issue report is about.

When looking into this myself I've had various "suspicions" regarding the node_modules folder within @openui5/ts-types-esm. I'm super happy that I wasn't totally wrong here and of course I was missing quite some context which thanks to you, I now have and understood. One key-aspect (for me) is the explanation from the TS dev team lead, that really helps a lot, great find.

We haven't made up our mind yet which way to recommend and not fully tested all implications, but at least it is now pretty clear what happens and what the available options are.

I'd say this issue should then be kept open as reference point or until it is either documented somewhere "more central" where it'd make sense or a recommendation can be given?

lemaiwo commented 2 years ago

Thank you @akudev !

Should we already change the template? Or do we wait for further recommendations from the UI5 team?

akudev commented 2 years ago

@wridgeu @lemaiwo We are going back to "typeRoots" for both the default @types folder as well as the UI5 types. And making sure the templates don't have a jQuery types dependency (which they shouldn't have had anymore, anyway).

Here I have added a summary of the issue in a more central place, so from my perspective this issue report could now be tied to the settings in THIS template (both jQuery and qUnit) and be closed once the dependencies are removed.

akudev commented 1 year ago

This is a sort of never-ending story. With TypeScript 5.1 the lookup behavior was changed when typeRoots is given. This leads to an error with the setup proposed above, roughly like this:

error TS2688: Cannot find type definition file for '@sapui5/types'.
  The file is in the program because:
    Entry point of type library '@sapui5/types' specified in compilerOptions

  tsconfig.json:16:9
    16         "@sapui5/types",
               ~~~~~~~~~~~~~~~
    File is entry point of type library specified here.

The current recommendation for tsconfig in TypeScript 5.1 onwards (but also usable in lower versions) is hence not to set any typeRoots, but instead to set types to the respective @openui5/@sapui5 ones plus all other used types. Or in case the standard-named @types/openui5 is used, not to set either.

prasanthkarukkuvel commented 1 year ago

Typescript 5.1 onwards to include @sapui5/types

    "types": ["@sapui5/types"],
     "typeRoots": [
        "./node_modules/@types",
        "./node_modules/@sapui5/types"
    ],
akudev commented 1 year ago

@prasanthkarukkuvel Well, actually - as mentioned above - the recommendation is not to set typeRoots, but instead to explicitly list the used types as e.g. done in the Easy UI5 template used in the tutorial:

"types": ["@openui5/types", "@types/qunit"]

Your suggestion, however, is not wrong and will also work, but one might still run into this problem. The official suggestion, on the other hand, means that one has to remember to add all additional types explicitly. So both ways have their advantages and drawbacks and we also had some discussions what to favor at the moment.

wridgeu commented 1 year ago

Hi @akudev, as this might be an ongoing topic (like it already has been now :D), maybe it makes sense to have an ongoing Github "Discussion" opened somewhere or have a tsconfig recommendation section (or something like it) added to https://sap.github.io/ui5-typescript/?

akudev commented 10 months ago

@wridgeu While the known issues already covered this topic, I have added an FAQ page with a section on it.

wridgeu commented 10 months ago

Awesome, thank you! Also good to know that there is a known issues section - never noticed 😅

BenReim commented 9 months ago

@akudev ~The issue still exists when using the sapui5/types with yarn (v3.4.1) and PnP.~

Edit: Turned out to be an issue related to the project's yarn setup.

akudev commented 9 months ago

@BenReim I have mentioned this problem in the second section of the known issues document.

akudev commented 4 months ago

For reference, another extended write-up of this topic with a bit more explanation than the compact "known issues" section: https://github.com/SAP/ui5-typescript/issues/461#issuecomment-2185831969