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

Redeclaration of Node_2 class causes type incompatibilty #1127

Closed
hschletz opened this issue Nov 27, 2024 · 6 comments · Fixed by #1142 · May be fixed by #1141
Closed

Redeclaration of Node_2 class causes type incompatibilty #1127

hschletz opened this issue Nov 27, 2024 · 6 comments · Fixed by #1142 · May be fixed by #1141

Comments

@hschletz
Copy link

hschletz commented Nov 27, 2024

I migrated my application to use the new dav module introduced in v3.10, but ran into a problem:

dist/dav.d.ts contains a redeclaration of the Node_2 class, which is a verbatim copy of the Node_2 class from the main module. Some functions, like resultToNode(), return an instance of the class from the dav module. This causes Problems with code that expects a Node or INode from the main module:

 error TS2345: Argument of type 'Node_2' is not assignable to parameter of type 'INode'.
  The types returned by 'clone()' are incompatible between these types.
    Type 'Node_2' is not assignable to type 'import("<project_path>/node_modules/@nextcloud/files/dist/index").Node'.
      Types have separate declarations of a private property '_data'.

As said, the declarations are 100% identical, but Typescript sees them as different types. There are some ugly workarounds:

  • Explicit cast: resultToNode() as unknown as INode (the intermediate cast to unknown is required to work around the perceived type incompatibility)
  • Change consuming code to allow a Node_2 from the dav module, but that looks more like an internal class name.

The redeclaration looks accidental to me. Should't there be only the Node_2 class from the main module?

Pseudocode to reproduce:

import { INode } from '@nextcloud/files'
import { resultToNode } from '@nextcloud/files/dav'

function test(node: INode) // same problem with Node
{
    return node
}

const node = resultToNode(...)
test(node) // results in above error
@skjnldsv
Copy link
Contributor

AH! I encountered the EXACT issue yesterday, couldn't fid the time to investigate :)
If you have a fix ready, feel free to send a PR 🙏

@skjnldsv
Copy link
Contributor

Guessing this is coming from #1118 ? 🤔

@hschletz
Copy link
Author

hschletz commented Nov 27, 2024

Sorry, this is beyond my skills... I think the problem comes from the way the types are imported in the dav code:

import type { Node } from '../files/node'

This seems to make the type part of the dav module, thus ending up replicated in dav.d.ts. Instead, dav.d.ts file should import the type from the main module, like this:

import { Node } from '@nextcloud/files'

Imports are generated only for references to external packages. I don't know how to do that for code from the same package, but that looks to me like a common task for developing library code.

It's not only the Node type getting replicated. Every import from the main module into the dav module would probably have the same problem.

@skjnldsv
Copy link
Contributor

skjnldsv commented Dec 10, 2024

Hey!

Can you try with this instead?

import type { Node } from '@nextcloud/files'
import { resultToNode } from '@nextcloud/files/dav'

function test(node: Node) {
	return node
}

const node = resultToNode(...)
test(node) 

@hschletz
Copy link
Author

Still does not work for me. I have no experience in testing local libraries. Anything wrong with these steps?

cd $HOME
git clone https://github.com/nextcloud-libraries/nextcloud-files.git
cd nextcloud-files
git checkout fix/imports 
npm run build
cd $HOME/my-project
npm install --no-save $HOME/nextcloud-files/

Creating your test code in my project directory, and passing some compatible argument to resultToNode(), still gives this TS error:

src/lib/test.ts:16:6 - error TS2345: Argument of type 'Node_2' is not assignable to parameter of type 'import("$HOME/nextcloud-files/dist/index").Node'.
  Types have separate declarations of a private property '_data'.

16 test(node)
        ~~~~

As you can see from the message, the module is correctly loaded from the local path.

My real code also gives the same error as before.

@skjnldsv
Copy link
Contributor

@ShGKme @susnux for when you are back 🙈
I don't get it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants