wallet-adapter
wallet-adapter copied to clipboard
Make package exports link to correct types in new TypeScript versions
Changes:
-
"declarationMap": true
allows IDEs to link to the source -
"inlineSources": true
makes it easier for external tools to read sourcemaps - removed
types
frompackage.json
on all packages. Node 16 module resolution does not read them, and other versions of node/typescript will find the co-located types
Tested this via yarn link
in saber-common. Feedback is welcome.
dumb question but does node v12 support this change?
I think I probably don't understand the original original problem that this PR sets out to solve. Is it just that command-clicking on a symbol in your IDE links you to the TypeScript definition file instead of the source code? I also feel a little bit dumb; if the problem is hyperclicking on code, where does Node module resolution come into play? The runtime dgaf about types, right? From the runtime's perspective, in production mode, everything is Just JavaScript™, and the only thing that follows types is tsc
and your IDE. Like, put another way, Node isn't resolving these imports, the TypeScript compiler is using the Node.js run-time resolution strategy to resolve these imports. From that perspective it doesn't matter what version of Node is on your computer, right? Right? ¯\_(°ペ)_/¯
I feel like I'm about to get schooled.
In other words, hitting go-to-definition on a declaration from a .d.ts file generated with --declarationMap will take you to the source file (.ts) location where that declaration was defined, and not to the .d.ts.
Oh what! I thought this was just something annoying about TypeScript and not something that you could configure. Yeah, that's way better.
When set, instead of writing out a .js.map file to provide source maps, TypeScript will embed the source map content in the .js files. Although this results in larger JS files, it can be convenient in some scenarios. For example, you might want to debug JS files on a webserver that doesn’t allow .map files to be served.
That definitely sounds like something we don't want to do (make JS files larger). What problem did this solve?
typescript mimics the node resolution for some reason instead of having their own one?
Not really clear why it does this myself.
https://www.typescriptlang.org/docs/handbook/module-resolution.html
That definitely sounds like something we don't want to do (make JS files larger). What problem did this solve?
makes debugging easier. We do this in our own code and i think its pretty standard to do it? It gets stripped out for production code.
Oh what! I thought this was just something annoying about TypeScript and not something that you could configure.
Yeah, it's way better.
Since you folks have done the legwork here already, can you just answer this: does TypeScript resolve modules differently depending on if the node
in your path is version >=16 or version <=15? I can only see in the TypeScript documentation that it tries to mimic the algorithm as “outlined in [the Node.js documentation].”
As for this GitHub issue, I wonder if maybe the title is confusing me as to its intent, and how the version of Node comes into play. Is the title of this issue maybe supposed to be more like ‘make symbols link directly to source code using hyperclick?’
I think I probably don't understand the original original problem that this PR sets out to solve.
Same. I haven't had a problem using this project in WebStorm, does it have problems in other IDEs?
@macalinao can you help provide more context? A few of these things (like inlining sources) doesn't really seem like something we'd want to do...
Since you folks have done the legwork here already, can you just answer this: does TypeScript resolve modules differently depending on if the
node
in your path is version >=16 or version <=15? I can only see in the TypeScript documentation that it tries to mimic the algorithm as “outlined in [the Node.js documentation].”
No-- it's based on the type of moduleResolution
in TSConfig, of which node
and node16
are two options.
As for this GitHub issue, I wonder if maybe the title is confusing me as to its intent, and how the version of Node comes into play. Is the title of this issue maybe supposed to be more like ‘make symbols link directly to source code using hyperclick?’
Yeah it's confusing-- I'll change it. Node16 support is one of the things this fixes.
I think I probably don't understand the original original problem that this PR sets out to solve.
Same. I haven't had a problem using this project in WebStorm, does it have problems in other IDEs?
@macalinao can you help provide more context? A few of these things (like inlining sources) doesn't really seem like something we'd want to do...
I think I probably don't understand the original original problem that this PR sets out to solve.
Same. I haven't had a problem using this project in WebStorm, does it have problems in other IDEs?
@macalinao can you help provide more context? A few of these things (like inlining sources) doesn't really seem like something we'd want to do...
Inlining sources makes debugging easier, but it'll make the NPM package bigger. The bundle size should be the same.
I can remove this if this is an issue.
Hey guys-- any updates on this? what can I change to get this merged in?
Closing in favor of https://github.com/solana-labs/wallet-adapter/pull/529