-
Notifications
You must be signed in to change notification settings - Fork 17
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
Import specifiers in --experimental-strip-types
#214
Comments
In cases where some
Would not be true, because non-compiled This is not as confusing as people think. Why should IIRC I implemented this behavior in ts-node: https://typestrong.org/ts-node/docs/options#experimentalresolver |
What about package consumers who don't care about TypeScript? If packages are published without a build step, they will be left on the side or they'll have to run their application with the strip-types flag only because of their dependencies. Also there's something that really needs a deep investigation: is the TypeScript type checker able to work with packages in node_modules that haven't gone through a build step that converts the
We are in deep disagreement here. I personally hate packages that are published as bundles. |
I made some local tests and it looks like |
@marco-ippolito Can you elaborate on this? Isn't |
There is a typescript flag that allows |
TypeScript does not allow to emit (transpile) code when this flag is enabled, so there's some level of inconsistency.
|
I believe this is something we can discuss with the TypeScript team as its a problem also shared by other runtimes |
@cspotcode The point of type stripping is to not need to use a build tool or bundler. Deno and Bun run TypeScript files directly, including in production, and our type stripping is intended to do the same. Yes this means that startup time is slightly impacted, but that’s not very concerning for long-lived servers, which are the most common use case for Node apps. And if you’re coding for a use case where startup time does matter, like a CLI tool or a serverless/lambda function, then you shouldn’t simply transpile files one-to-one, you should bundle them like you would for the browser.
@targos When I wrote that I was thinking of applications, not packages. But to your point, I think what we hate about packages published as bundles is when they bundle their dependencies, inlining them rather than treating them as external dependencies that can be deduped. I don’t think it’s terribly offensive when package authors use a build tool to optimize their library while externalizing dependencies, so that my app loads Maybe you’re also offended by package authors who minify or otherwise make their packages hard to debug at runtime, but ultimately you’re the one who’s choosing whose open source packages to use, and you can evaluate them based on whatever criteria you want. The Svelte team writes apps in TypeScript and libraries in JavaScript with JSDoc types, so that their libraries don’t need a build step yet can be consumed by anyone while still getting type checking, and that feels like the best model to me. If we're going to recommend anything, that’s what I would recommend.
This is the same argument as any new or nonstandard syntax. We had this same discussion around Generally public package authors don’t want their libraries to be hard to use, or they won’t attract many users. And private package authors, like corporate users writing packages for specific teams whom they know and can coordinate with, might choose to opt into experimental features since they know their users can handle it and want to. It’s not really our place to intrude in the relationship between package authors and consumers. While I agree in principle that we should do what we can to prevent users from shooting themselves in the foot, where I draw the line is that I don’t think we should intentionally limit Node, blocking perfectly valid and unobjectionable use cases, just because some package authors might misuse a feature. By that logic we shouldn’t have shipped |
Also, the cat is already out of the bag: Bun evaluates TypeScript within mkdir temp && cd temp
mkdir -p ./node_modules/pkg
echo 'export const message: string = "Hello!"' > ./node_modules/pkg/index.ts
echo 'import { message } from "pkg"; console.log(message)' > test.ts
bun test.ts
Hello! |
This is the biggest problem for me. TypeScript does not have a spec, but it has a reference implementation. If we evaluate code as |
From #217 issue:
For distribution, if a developer cares about performance, he should provide already compiled Running It would be fine, if @microsoft TypeScript team will Currently, it has the follow description:
That requirement is unacceptable for library projects that use I think, if the Here is a library config to compile "module": "NodeNext",
"moduleResolution": "NodeNext",
"noEmit": false,
"declaration": true, It works, but it requires to use Adding
I just say it again: I think, if the Why do these limitations exist?
Can't TS just replace |
No, because it might be a package named See also nodejs/node#53725 (comment) |
I'm very excited about However, the Current proposal (which seems to be strongly supported by @GeoffreyBooth and @marco-ippolito) is the single reason that makes For me to use type stripping in practice, the behavior described in "Replace .js alternative" would be needed: Imports specifying a The reason is that my libraries all use ESM and are all written to be compiled by I really hope that type stripping can support
Being usable is way more important than being intuitive.
This seems like a non-issue in practice. Projects will either have It has also been argued in the original PR that extra checks for file existence implies a performance hit. I'm perfectly willing to accept the cost of an extra check for a @GeoffreyBooth seems to have a more expansive vision: Everyone ships TS files directly. I don't think this vision is realistic to achieve in the medium term for reasons I won't elaborate on here. I just want to note that in my advocated solution there is no performance hit when loading
Any serious library will always use For me, the point of type stripping is to not have to use any build tool other than But using no other build tool than
I agree with @targos here, I also hate packages published as bundles. And yes, I am indeed also offended by package authors who minify or otherwise make their packages hard to debug at runtime. And I personally want to publish libraries that are not minified or bundled. That's why I like to only use
I don't think it's the Node.js TSC's job to recommend any particular opinionated way of writing libraries. I fully respect Svelte's style of doing things, but it should be out of question that writing TS source files is also a valid approach. Using TS for libraries is the style that I and apparently most people prefer, so it should clearly be considered in a discussion of how to support TS in node!
I couldn't agree more! That's why I recommend resolving |
@mitschabaude it appears the TypeScript team are considering options to resolve this in tsc |
@JakobJingleheimer I'm moving our discussion here from #217 (comment) In general I get the argument that "guessing an extension" is a step backwards for Node.js. Nonetheless it's a much lighter form of guessing than we used to have in CJS. Also, thanks @JakobJingleheimer for pointing out that rewriting the imports can also be accomplished using a Node.js resolve hook. That might become my method of choice for now. There's just one point where I think you and other Node.js team members are a bit off: with the recommendation to "just use
|
It might not be as complicated as originally thought: microsoft/TypeScript#59597 (comment) |
that comment was written before the meeting that I linked to |
I would also like to thank the Node.js maintainers/community for the work in this area! However, I would like to second the opinion of @mitschabaude in regards to the .ts/.js imports. From my understanding, could this not be partially achieved with an ESM hook? Maybe only conditionally active when This could of course be implemented in user space, but then I feel like the usability of the Node.js TypeScript integration is unnecessarily limited. Thank you for your consideration. Naive implementation: Codeimport { existsSync } from "node:fs";
import { basename, dirname, extname } from "node:path";
const typeScriptExtensions = [".ts", ".mts", ".tsx"];
/**
* Example resolve for resolving .js imports from TypeScript files
* that point to other TypeScript files, instead of .js files.
* If resolving fails, we check whether there is a corresponding
* TypeScript file.
*/
export const resolve = async (specifier, context, nextResolve) => {
try {
return await nextResolve(specifier, context);
} catch (err) {
if (
!err.url ||
!specifier.endsWith(".js") ||
!context.parentURL ||
!typeScriptExtensions.includes(extname(context.parentURL))
) {
throw err;
}
const extensionlessUrl =
dirname(err.url) + "/" + basename(err.url, extname(err.url));
for (const typeScriptExtension of typeScriptExtensions) {
const maybeTypeScriptFile = extensionlessUrl + typeScriptExtension;
if (existsSync(new URL(maybeTypeScriptFile))) {
return {
format: "module",
shortCircuit: true,
url: maybeTypeScriptFile,
};
}
}
throw err;
}
}; Playground (You might need to switch to the zsh Terminal and run |
It absolutely can and has been ;) An argument about how difficult it would be for |
As previously stated, this is complicated. |
@kyubisation I took the liberty of reviewing your proposed loader:
As is, it would still have some inconsistencies with
If I had to fork one of the project to "make it work somehow", I would pick Node.js, but that's because I'm more familiar with the codebase (i.e. I can't really speak for how easy/hard it would be to implement in
I'm hopeful the |
As mentioned:
But nevertheless, thank you. I would be open to contribute, but as you mention, there is little point until consensus is reached.
So, my comment is just meant as another feedback from a potential and interested consumer. |
My comment might have been a bit harsh. The Node.js is doing the best they can. |
Guys I'm not sure if this is the right discussion, but I have 2 questions about the flag.
import foe from "./foe"; it fails, since I'm required to provide an extension and I'm reluctant to migrate my codebase fully to ".ts" extensions everywhere: if anything happen I can't switch easily back to
|
Stripping types is far simpler than what you're asking. Please read the rest of this thread, such as the posts immediately above yours, for information about your question 🙂 |
When Node.js is run with |
In today's TypeScript team meeting emerged that TypeScript will create an emit flag that rewrites the |
@marco-ippolito what do you think is the best way to handle package imports? // package.json
{
"imports": {
"#dep": "./path/to/dep.ts"
}, Would there be a way to re-write these paths as part of |
Sorry, what's the problem with subimports? I just added a case for them in my codemod, and they seem to work fine out of the box. |
I think the most straightforward thing here is to make a condition specific to your project such that when it's enabled, it points to the source, but otherwise points to the emitted JS. (e.g. in the same vein as https://colinhacks.com/essays/live-types-typescript-monorepo?q=1 or https://github.com/arethetypeswrong/arethetypeswrong.github.io/pull/194/files#diff-0b810c38f3c138a3d5e44854edefd5eb966617ca84e62f06511f60acc40546c7R30 is an example of this working for import conditions. |
Here's the PR to watch: microsoft/TypeScript#59767 Currently it says this:
I'll keep my fingers crossed 😅 |
Interesting case, can you open a separate issue on the nodejs/typescript repo? I think this is worth discussing |
fwiw, I've already been using |
Hey guys, I made a repro here => https://github.com/damianobarbati/strip-types-repro For anyone looking for an all-in-one repro on how to have the following all working together:
cc @marco-ippolito @electrovir @aduh95 @JakobJingleheimer 😈 |
Is that What's missing from your repo IMO is creating compiled JS output (tsc doesn't handle that because of |
I think @jakebailey 's comment links to good solutions for this. The package-scoped condition seem simplest: "exports": {
".": {
"@<package-name-here>/source": "./src/index.ts",
"default": "./dist/index.js"
}, Part of me wishes there was an environment variable (more granular than |
As a library maintainer who wants to run source TS files during development but distribute javascript for production; I have been experimenting with conditional imports + a bundler for production/distributed files // package.json
{
"name": "my-lib",
"type": "module",
"exports": {
".": {
"source": "./src/lib.ts",
"types": "./src/lib.ts", // Is it valid to use typescript sources as types?
"default": "./dist/lib.mjs"
}
}
} During development I run:
// ./test/consumer.ts
console.log(await import('my-lib')) And to build I run:
Though this gets complicated when you need to use worker threads. For this I use // package.json
{
"import": {
"#worker": {
"source": "./src/worker.ts",
"default": "./dist/worker.mjs"
}
}
} import * as url from 'node:url'
new Worker(url.fileURLToPath(import.meta.resolve('#worker'))) And for // package.json
{
"bin": "./bin.js",
"import": {
"#bin": {
"source": "./src/bin.ts",
"default": "./dist/bin.mjs"
}
}
}
} // ./bin.js
await import('#bin') With Nodejs having a built-in test runner and typescript support, I can drop a bunch of dependencies, which is nice for project longevity. Looking forward to eventually being able to use |
im confused; non-conditions objects’ LHS are all required to start with a dot, to prevent precisely the scenario of redirecting a package import to a dependency. did that change at some point? |
Within that block, the property names are the condition names, not names of imports; the implication is that you write this when you want to run from source (taken from the ATTW PR): $ node --experimental-transform-types --conditions=@arethetypeswrong/source --test 'test/**/*.test.ts' (Using just "source" itself is potentially problematic because you can end up in the situation where a package you don't own may publish also with the condition "source" and now you're running someone else's source.) |
Gotcha, that’s clever. |
TypeScript PR for .ts imports is merged 🚀 Woohoo! |
I have tested It's now possible to use Also, I can now run However, currently there is a bug that imports in generated And it does not support aliases, even while the alias is resolved to a relative path. |
I wrote a small package to install a Node resolution hook so |
You might want to take a look at |
Yeah sure, but there are still caveats around that as mentioned in the link, which aren't a problem with the Node resolution patch. That'll certainly make life a lot easier, and is a huge backflip considering how hard they've stuck to their guns on rewriting import extensions in the past, but I think there are plenty of cases it'll still make sense to patch Node like this. Whatever works best for everyone, of course. Ideally we don't need glue packages like this 🤷♀️ |
This is a dedicated thread for the issue of file extensions in
import
specifiers in TypeScript code. Refs: nodejs/node#53725 (comment), #208 (comment), nodejs/node#53725 (comment), nodejs/node#53725 (comment), among others.Current proposal: Users write full filenames with extensions in their specifiers, similar to ES module JavaScript:
import './file.ts'
. Also,.ts
extensions are required inrequire
calls, similar to the.cjs
extension in CommonJS JavaScript:require('./file.ts')
.tsc
via theallowImportingTsExtensions
tsconfig.json
option.tsc
cannot compile such TypeScript files into JavaScript (today) but many other build tools can. However, there’s not necessarily a need to compile TypeScript files into JavaScript when they can be run directly. The only benefit that a build step would provide would be for startup time performance (it would be slightly faster to evaluate JavaScript directly rather than stripping types from TypeScript and then evaluating it) and users concerned about this should arguably be using a full-featured bundler such as Vite rather than a one-to-one transpiler such astsc
.Extension searching alternative: Users would leave off extensions, similar to CommonJS:
import './file'
. This is a very common way for TypeScript code to be written today, because most TypeScript code is transpiled down to CommonJS JavaScript where such specifiers are permitted.require
statements, as it would be a semver-major breaking change.Replace
.js
alternative: Users would writeimport '/file.js'
to refer tofile.ts
. This is whattsc
recommends for users usingtsc
as their build tool, as it compiles.ts
files to.js
files one-to-one and never rewrites import specifiers..js
files generated as part of running files viastrip-types
.file.js
andfile.ts
exist? None of the various options (error, load one file or the other) are obviously correct or consistent.file.ts
tofile.ts
is arguably something that’s logically handled by the build tool or bundler, as that’s the tool generating thefile.js
file to which the new import specifier would refer. It’s hard to see how it’s Node’s responsibility to make up for a shortcoming in a TypeScript build tool.The text was updated successfully, but these errors were encountered: