Closed TomAFrench closed 2 months ago
Related to #462
default
needs to come last else it will throw an error
Yea the types in this package doesn't work. In my vscode it says:
Could not find a declaration file for module 'threads'. '/home/cmcdragonkai/Projects/js-workers/node_modules/threads/index.mjs' implicitly has an 'any' type.
There are types at '/home/cmcdragonkai/Projects/js-workers/node_modules/threads/dist/index.d.ts', but this result could not be resolved when respecting package.json "exports". The 'threads' library may need to update its package.json or typings.ts(7016)
This with native ESM support.
When I was using CJS, I could directly import the type files, hacking around the dist
. But with ESM, I think you need to specify the types
key properly above require
, and default
... etc.
These are the types I care about:
import type { ModuleThread } from 'threads';
import type { ModuleMethods } from 'threads/dist/types/master';
import type { QueuedTask } from 'threads/dist/master/pool-types';
They used to work when I was using CJS. Now I'm transitioning to ESM, and the last 2 imports no longer work.
@Altrue @tien Ah, thank you. I must admit I didn't test that as a required ordering to fields in JSON doesn't really make sense.
I can't quite remember why I was running into this issue anymore (most likely a side project I'm not particularly active in Ah, this was for work but we've gone down a different path) so this isn't a huge priority to me anymore and @andywer doesn't seem to be actively merging PRs currently.
@andywer are you still working on this? Without this PR I think ESM downstream projects can't use this.
And I actually care about these types: https://github.com/andywer/threads.js/pull/470#issuecomment-1676248028
Hi @andywer, I've ran into the same issue, fixed them with the changes from this PR (using patch-package). Would be great if you could merge it 🙏
Hi @Anton-Plagemann & @TomAFrench, sorry for the unresponsiveness!
Will merge right away.
Tests are broken, unfortunately. Updated some dependencies on https://github.com/andywer/threads.js/tree/chore/fix-ci, but still have an issue with a worker in a browser. The statically deployed worker script doesn't exist anymore… 😕
@andywer hi. is there anything that needs to be done to speed up the process so that we can get a new release with this fix in npm?
@oxcl If someone could fix the build, that would be amazing. I could also build and publish despite the failing test, but we should definitely fix this in the mid-term.
@oxcl If someone could fix the build, that would be amazing. I could also build and publish despite the failing test, but we should definitely fix this in the mid-term.
@andywer I just ran npm audit fix
on the project and it seems like the problem went away.
both npm run build
and npm run test
run successfully. should i submit a PR?
@oxcl Very hard to believe. The worker script deployment doesn't exist anymore: https://github.com/andywer/threads.js/blob/5f9a15f25bf6b4e8c79c51a63f6757c5b28d0f50/test-tooling/webpack/app.ts#L41
Unfortunately, I also don't know if I have the code anywhere else, but I think it was just the transpiled and bundled hello-world.ts
worker IIRC. Maybe we don't even need bundling anymore and can just use ES module imports from esm.sh or so.
I'm experiencing a lot of issues when trying to resolve the types
The last error explicitly calls out that changes need to be made to the threads.js
package.json
. I've then added a "types" field for each of the conditional exports.