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

Please ship javascript and .d.ts. files #81

Closed
t-animal opened this issue Mar 20, 2023 · 6 comments
Closed

Please ship javascript and .d.ts. files #81

t-animal opened this issue Mar 20, 2023 · 6 comments

Comments

@t-animal
Copy link

t-animal commented Mar 20, 2023

Compiling of dependant projects fails, if they have a stricter tsc config (e.g. enable noPropertyAccessFromIndexSignature and you get

node_modules/generouted/src/core.ts:66:43 - error TS4111: Property 'index' comes from an index signature, so it must be accessed with ['index'].

66         parent?.children?.[insert](route?.index ? route : { path, ...route })

This could be improved by shipping compiled javascript-code and .d.ts typings. That would also allow people not using typescript to use the library.

See also microsoft/TypeScript#47387 (comment)

@oedotme
Copy link
Owner

oedotme commented Mar 20, 2023

Hey @t-animal, thanks a lot for the suggestion and providing the reference.

As generouted targets Vite only currently, users are able to use TypeScript out-of-the-box with Vite without any installation needed.

It was fine to ship it as TypeScript and Vite handles it automatically. You'd need to install TypeScript for type-checking, etc.

That being said, I would definitely look into it and overall might end up better shipping js for many reasons. Would close #72 as @spa5k also suggested.

Will close this issue once updated.

@t-animal
Copy link
Author

Stuff like this is pretty much exactly why I'm not happy about the general adoption of esbuild, because it just is not a typescript compiler (not sure if they still claim that). Anyways, when bootstrapping a react with vite and the react-ts template, it (wisely) includes a tsc step in the build-script. https://stackblitz.com/edit/vitejs-vite-xpresx?file=package.json%3AL8&terminal=dev So this problem here affects anybody who enables noPropertyAccessFromIndexSignature in their tsconfig

@spa5k
Copy link
Contributor

spa5k commented Mar 20, 2023

Ping if you need help in doing so, meanwhile I'll check on my own and see if we can do it.

@ArnaudBarre
Copy link

Does this can be fixed by shipping TS & dts files?
I like the idea of publishing TS source as the cost of transpiling is very cheap now. But if this issue cannot be fixed with typing files, I think that the inability of current TS version to skip node_modules with raw TS files is a big blocker for pushing the ecosystem in that direction as this would be really painful for user using stricter version of TS than library authors.

Hoping for a clear response of the TS team on that here

@oedotme
Copy link
Owner

oedotme commented Mar 21, 2023

Hey @ArnaudBarre, thanks for your input, tbh I thought ts files inside node_modules are ignored from type-checking by default.

I suspect using ts + dts would result in the same, I'll need to try it out tho. In any case whether with or without transpiling, an extra pipeline will be added anyways. I'm not a big fan of that, but it might solve couple of issues for now.

I also like Bun and Deno approach, I started this library as one source code file without transpilation or build step which was a very nice author experience and made it super easy to iterate fast.

I subscribed to your comment to get an update. Thanks again!

@oedotme
Copy link
Owner

oedotme commented Apr 10, 2023

Initial bundling was introduced in #82, but using stricter config was resolved in a later commit to bundle the core bb218aa. So right now, from v1.12.1 and above, the packages are published as .js + .d.ts files.

@oedotme oedotme closed this as completed Apr 10, 2023
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

No branches or pull requests

4 participants