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

added module declaration #879

Merged
merged 3 commits into from
Jul 24, 2023
Merged

added module declaration #879

merged 3 commits into from
Jul 24, 2023

Conversation

peeveen
Copy link
Contributor

@peeveen peeveen commented Jul 23, 2023

Suggested fix for #878

Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could you add a typescript test for this? Just to check it's working as expected

@peeveen
Copy link
Contributor Author

peeveen commented Jul 24, 2023

Could you add a typescript test for this? Just to check it's working as expected

I spent a few hours trying this. I tried to create a whole other test "project" inside the test folder, with it's own package.json with "type": "module" in it, and make a relative local reference to the parent aedes package as a dependency, which became a circular folder symlink/shortcut ... but I think node/tap/npm/tsc/something is getting confused, because I can't get my test code to stop complaining about "cannot use namespace as type" unless I add "type": "module" to the main project package.json ... which causes all manner of other problems.

Basically, I gave up.

But adding those three lines into the node_modules/aedes/aedes.d.ts file in my project is making everything work beautifully.

If the lack of an internal test is a big problem, just reject this. I'll keep using the workaround.

Copy link
Member

@robertsLando robertsLando left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's ok, no worries

@robertsLando robertsLando merged commit f6387a8 into moscajs:main Jul 24, 2023
@peeveen peeveen deleted the module-878 branch November 19, 2023 20:03
@hjdhjd
Copy link
Contributor

hjdhjd commented Nov 23, 2023

This PR breaks TypeScript modules with the following error:

node_modules/aedes/aedes.d.ts:11:3 - error TS2666: Exports and export assignments are not permitted in module augmentations.

In general this isn't a good idea, as I understand it. As of right now, Aedes is unusable in even the simplest of TypeScript projects.

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

Successfully merging this pull request may close these issues.

3 participants