chore: build with rollup

[benmccann]

The part I'm not quite as sure about is the types generation. I don't have a lot of experience doing that with rollup. It should also be noted that I don't think the existing build:types was actually used as it was outputting to the lib directory, which the package.json didn't refer to.

Please feel free to make any additional changes to clean this up. I just thought I'd send this as a start

[43081j]

build:types was used just to run a type check (basically equivalent of --noEmit since we didn't use the output)

esbuild won't type check, but I guess the dts generation plugin tsup uses will

[43081j]

we should still end up with both of these (main.d.ts and main.d.cts) for typescript to be happy

what does rollup's typescript plugin output?

[benmccann]

I'm not exactly sure how to do that with rollup's typescript plugin, but I don't think it's necessary to have both since they have identical contents

[43081j]

It is necessary to have both. If the package type is module, typescript treats all dts as esm types and vice versa. Requiring CTS for cjs modules

So they will be the same contents but hasn't to exist twice (TS is doing the right thing here, we're all wrong for making dual packages a thing 😅)

[ghiscoding]

hey guys I recently found out that "Are The Types Wrong" website have also created a CLI @arethetypeswrong/cli which allow you to test and run locally even before publishing (there's actually also the upload button on the website to upload a packed file that is also possible).

@43081j are you sure that just cloning a TypeScript d.ts file to .d.cts with same content is enough? I think that I tried that before and the CLI above didn't pass, but maybe I've done something wrong!? Side note, I tried with plain tsc, not sure if the Rollup/esbuild DTS plugins bundles dts differently though, it looks like they do compared to tsc

[43081j]

yes, attw will also test for this

basically you should have two exports in your package.json: one for import and one for require. each has its own entrypoint and its own types if the file can't be inferred

you can see an example in tinyexec:
https://unpkg.com/browse/[email protected]/package.json

attw is happy with this, whereas if we didn't have the cts and reused the same dts, it would complain about masquerading (correctly)

[ghiscoding]

but you're using tsup and yes I got it working with it, but if I use plain tsc and just copy/rename the d.ts to d.cts and update the import/require as you shown in tinyexec then it doesn't work. What I haven't tried though is to move esm/cjs into their own folders, that might avoid the masquerade detection of attw. Thanks for the info anyway

[43081j]

tsup or not, you need two copies of the dts file (preferably a dts and a cts if the overall package is an es module)

I'm not sure why it didn't work for you. You can run tsc twice or copy the files and achieve the same result. I've done that in other packages without tsup

Overall point being you can't use one dts file for a dual package, as that would be interpreted as the root type (e.g. esm if type is module)

[ghiscoding]

I figured out my problem, I wasn't using .js extensions in my imports and when testing my hybrid approach with type=module and passing it to attw it was saying that my files were the wrong format. So anyway that's fixed by using extensions and for the d.cts file, I simply added a simple Node script via clone-dts.mjs

// clone-dts.mjs
import { readFileSync, writeFileSync } from 'node:fs';

writeFileSync('dist/index.d.cts', readFileSync('dist/index.d.ts'));

isn't this the only thing missing for this PR (reverting back to the original exports)? There's probably a way to do it with a Rollup plugin, but we probably don't need to go crazy about it, my script works great with && node clone-dts.mjs in my build script. I guess this rollup-plugin-copy would also do the work

[43081j]

you would have to copy both the source and the DTS file

js/DTS and cjs/cts (2x everything)

I've lost track of what we're discussing here too since we've gone on a tutorial-like tangent 😅

basically we need to make attw happy and we do that by having 2x everything. how you achieve that, I don't think matters much, whatever works. I don't think Ben has looked at this for a while anyway now, I'd probably have to pick it up at some point

[ghiscoding]

ah sorry, I just said that I got it all working on my end and I was actually discussing this PR in my last comment. I should have been more clearer 😆

[jerome-benoit]

What is the rationale for switching to rollup, which is quite heavy compared to tsup for such a small code base?

[43081j]

iirc it was just an attempt to fix the problems we had with cross spawn being transpiled by esbuild

I think we should actually try move away from cross-spawn and have our own code to do the parts of it we use

currently, we do this filthy hack to get esbuild to play nice:

tinyexec/tsup.config.ts

Line 17 in 2adbaf7

return { js: `import { createRequire as __tinyexec_cr } from "node:module";const require = __tinyexec_cr(import.meta.url);` };

but really just need to stop depending on a commonjs dependency

[jerome-benoit]

iirc it was just an attempt to fix the problems we had with cross spawn being transpiled by esbuild

Fixing cross-spawn to properly export for CJS and ESM is less complicated.

[43081j]

we use such a tiny amount of cross-spawn that it isn't worth pulling it in

we should just be writing our own code for the part we care about IMO

much of the function we call from it has a lot of logic we don't really need, and all other functions are unused

[benmccann]

Ah, interesting. Looks like it's just the parse method that's used:

https://github.com/moxystudio/node-cross-spawn/blob/77cd97f3ca7b62c904a63a698fc4a79bf41977d0/lib/parse.js

[43081j]

We basically use it just for the sake of resolving commands to their paths on windows

So foo becomes something like C:/wherever/foo.cmd

Ultimately it then calls cmd /aBunchOfFlags C:/wherever/foo.cmd iirc

I feel like there will be more edge case logic in there we don't actually care about too, and could just document instead