Skip to content

Conversation

@remyguillaume
Copy link

Hi @WebReflection,

This should solve the problem described here: #143
I don't know if you want to upgrade the version number, I'll let this up to you.

Please tell me if something is missing.

Thanks!

@WebReflection
Copy link
Owner

I don't think those files are created though ... if you npm run build can you see those?

@remyguillaume
Copy link
Author

remyguillaume commented Jan 9, 2025

Yes, they are :
image

Actually, I think they are created with npm run build:types

@remyguillaume
Copy link
Author

Oh, wait wait, I've notived another typescript issue. I'll try to fix it in the Pull-Request as well if you're ok.

@WebReflection
Copy link
Owner

I read .d.ts there ... nothing about .d.mts or .d.cts ...

@remyguillaume
Copy link
Author

Yes, that's exactly why I've proposed this change.
In the current version of the package.json is looking fo those mst and cts files that just do not exist. I've changed it to use the .d.ts files.

Those files do not exist in the library :
image

...
But I think I missunderstand the error. Actually there is a script (rollup/ts.fix.js) that should be executed to create the cts and mts scripts. But it is not, this is probably where the error is.

I'll investigate it, sorry I probably missunderstood something here.

You are actually not using typescript for the whole project, are you?
The typescript definitions are just here for convenience for the people using your lib and working with typescript?

@WebReflection
Copy link
Owner

WebReflection commented Jan 9, 2025

Definitions are either automatically generated or manually maintained… the project is a JS one … if you check previous issues are all about TS but because I’m rewriting this whole library for hopefully the last time I don’t care much about fixing this but I’d welcome a fix that works

@WebReflection
Copy link
Owner

More context here: #141

@remyguillaume remyguillaume reopened this Jan 9, 2025
@remyguillaume
Copy link
Author

Ok, Ive understand the problem. It is linked to the typescript version.

In this commit :3860505#diff-7ae45ad102eab3b6d7e7896acd08c427a9b25b346470d7bc6507b6481575d519R42, the version of typescript has been changed.

Since then, there is an error with the command npm run build:types which raises an error. This error is somehow hidden during the build process, and the error seems not to be raised.

With the new version of typescript, the line export const asElement = ({ [nodeType]: type }) => type === ELEMENT_NODE; cannot be unsterstood any more by typescript.
So I've change it to something more simple, and now it's working again, all the .d.cts and .d.mts files can be generated correctly.

I've also reverted my other changes, there are not needed.

@remyguillaume
Copy link
Author

It appear that the same problem was already solved in another Pull-Request : #142.

Perhaps you can close this one and merge the other one ? Or merge this one and just close the other one, as you wish :)
Have a nice day !

@WebReflection
Copy link
Owner

wait .. how is that related? 🤔

@remyguillaume
Copy link
Author

wait .. how is that related? 🤔

Hum... Not sure I understand your question...
Actually it's exactly the same line of code... But 2 different solutions.

@eatsjobs
Copy link
Contributor

related: #145

@devinivy
Copy link

devinivy commented Mar 9, 2025

As mentioned above, it looks like the types aren't in a working state right now since the build step only partially completes due to a type error, which is addressed by this PR. I wrote down some details in #143 (comment), the main issue being:

So what's happening here is that the build step is partially failing, and as a result the contents of the types/ directory doesn't match the type files referenced in package.json.

@WebReflection
Copy link
Owner

did latest TS related MR fix this issue too?

@WebReflection
Copy link
Owner

no answer in weeks ... I take as a yes.

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.

5 participants