Skip to content

Design Meeting Notes, 6/15/2022 #49579

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

Closed
DanielRosenwasser opened this issue Jun 16, 2022 · 6 comments
Closed

Design Meeting Notes, 6/15/2022 #49579

DanielRosenwasser opened this issue Jun 16, 2022 · 6 comments
Labels
Design Notes Notes from our design meetings

Comments

@DanielRosenwasser
Copy link
Member

More-Specific TemplateStringsArray

#49552

  • We have a sort of gap in what you can express between function calls and tagged templates.
  • Some things that are lacking
    • Being able to infer specific values and raw values in place of a more-broad TemplateStringsArray type.
    • Inference in the face of intersections.
  • Why? Why now?
    • Has come up in a few issues.
    • Relatively little work.
    • Wanted to power more powerful mechanisms using plug-ins in tsc, but the type-system might be powerful enough now.
    • It's an asymmetry with function calls.
  • We said don't do this (Design Meeting Notes, 8/18/2021 #45504). The type system isn't optimized to be a parser. Function calls maybe provide enough "syntactic salt". So why do we want to incentivize this?
    • You can already do this today.
    • Also, language service plugins can give you errors, but you can't get a better type.
  • Tuple inferences may be good regardless of this change.
  • Can give more specific return types, can validate the string, and even use inference to validate the string.
    • Should make sure the DSL libraries we're trying to model here are actually tested.
    • But also, the error messages that these produce will be totally unappetizing.
  • Conclusion
    • Not 100% sold across the team, but we can convince ourself that it's reasonable to bring in. Not a priority for TS 4.8, but if it gets in, it gets in.

A Tour of Module Resolution Strategies

https://gist.github.com/andrewbranch/3020c4e24092bd37f7e210d6f050ef26

  • Node.js 16+ ESM support was a good focus, but lots of other resolvers that have a non-consistent set of features.
  • Been telling people "you need to reference .js as a module extension" for Node.js.
    • If that's too painful, but maybe you shouldn't use the new mode if you're not using Node - but then what should people be doing?
    • Usually the best, most-maximal one is just called, uh, node.
  • People using bundlers are often best-off targeting --moduleResolution node.
    • But then every bundler supports the exports field.
    • But then our old node mode doesn't understand that.
      • Maybe we should pick that into node?
        • Seems like a "duh!" but then...why don't we see lots of demand for this!?
          • Hypotheses:
            • Relatively uncommon to use exports.
            • ESM/CJS APIs are similar enough.
            • People don't know how to ask, especially because this is so confusing.
  • This is all very confusing - if people are so frustrated, couldn't we transform paths based on .ts to .js?
    • Always comes up - but it amounts to a transformation from a "TypeScript-specific module system" to several target module systems, and they need to be able to compose well.
      • How do you deal with lies in .d.ts resolution?
      • Do the .ts extensions get preserved in .d.ts files?
      • How does it deal with dynamic imports? Do you need a TS dynamic import helper for non-static imports?
      • How do you refer to dependencies in packages which are exposed as .js files? They aren't .ts files.
  • The point of this table is not to try to model every single bundler behavior, it's to try to figure out the common module targets and where we have a gap.
  • So one of those common targets is places where you don't need a compiler and the runtime/resolver can do the TS -> JS transform (e.g. Deno, bundlers, etc.).
  • No conclusion, but at least feel like we're getting a better understanding of the lay of the land.
@DanielRosenwasser DanielRosenwasser added the Design Notes Notes from our design meetings label Jun 16, 2022
@milesj
Copy link

milesj commented Jun 16, 2022

The module resolution is extremely confusing, and this doesn't even take into account tools like ESLint that traverse the file system (import/no-unresolved, etc). When a TS file has imports with an extension that point to a non-existent file, it makes this very hard to understand.

What if the extension in imports were ditched, and instead assumed/derived by compiler options?

  • moduleResolution: nodenext + module: commonjs === .cjs (.cts lookup)
  • moduleResolution: nodenext + module: es/nodenext === .mjs (.mjs lookup)
  • moduleResolution: node + module: commonjs === .js (.ts lookup)
  • moduleResolution: node + module: es/nodenext === .js (.ts lookup)

Or if that is still too confusing, being explicit with an outExtension compiler option is probably the path of least resistance.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jun 16, 2022

That just moves the problem elsewhere and introduces more configuration. You can't unconditionally add file extensions -- importing from fs/promises should not print fs/promises.js, that won't work. Yet people can and do use path mappings combined with bundler configuration to make paths that look indistinguishable from that:

import 'vs/editor/editor.all';

so now we need some additional configuration that controls which imports of the form 'a/b' get rewritten to including file extensions. But it's worse - is the file editor.all.ts, or editor.all/index.ts? Now there's more information from disk to fetch.

All of this configuration effectively takes us backwards in the journey to making sure files are transpilable without needing extra information from the file system or from other files in the system, which is a key source of performance gain in a ton of scenarios including extant bundlers like esbuild, and our own ability to parallelize emit in the future.

@Josh-Cena
Copy link
Contributor

Josh-Cena commented Jun 17, 2022

but then...why don't we see lots of demand for this!?

Please add "we hand-write ambient declarations" to the "hypotheses" 😅 https://github.com/facebook/docusaurus/blob/5746c58f4145f56b92512095bce71428638d9d45/packages/docusaurus-plugin-content-docs/src/plugin-content-docs.d.ts#L620-L622

Now we compile with NodeNext (so we don't hand-write these declarations), but when we import some package with type: module, we have to add @ts-expect-error.

@lemonmade
Copy link

Hey folks, thanks so much for making these meeting notes public like this! It provides wonderful transparency to the community. I learned a lot from reading this issue and the linked gist. ❤️

I don't have much in the way of useful solutions to add, but I wanted to share my own confusion with TypeScript's moduleResolution flag. I maintain a project, Quilt, that allows you to build modern, ESM-focused Node packages in TypeScript.

This project uses Rollup to build package outputs, and TypeScript is just used to produce type declaration files. It supports building packages with multiple entry-points using the following kind-of-awkward pattern where, in addition to exports fields for defining the entries, you use a combination of the top-level types and typesVersions to expose the entries for TypeScript consumers:

{
  "exports": {
    ".": {
      "quilt:from-source": "./source/index.ts",
      "quilt:esnext": "./build/esnext/index.esnext",
      "import": "./build/esm/index.mjs",
      "require": "./build/cjs/index.cjs"
    },
    "./testing": {
      "quilt:from-source": "./source/testing/index.ts",
      "quilt:esnext": "./build/esnext/testing/index.esnext",
      "import": "./build/esm/testing/index.mjs",
      "require": "./build/cjs/testing/index.cjs"
    }
  },
  "types": "./build/typescript/index.d.ts",
  "typesVersions": {
    "*": {
      "testing": [
        "./build/typescript/testing/index.d.ts"
      ]
    }
  }
}

These can get pretty annoying to maintain — here’s the most complex version of this pattern in the monorepo — so I was really excited by the news that a types field could be added to existing export declarations instead. However, I personally couldn't get over the need to change relative imports to import from their compiled output locations, rather than from the .ts source files. I think for me that mostly comes down to that being different from how browser and deno resolution work. I am definitely open to biting the bullet and switching all relative imports to include extensions, as that seems like the direction everything is heading, but I am not sure I can get there with having to import from a non-existent relative path, personally. This tool produces no .js files, and it does not produce outputs beside the source, so I'd always be bothered by the import paths pointing to files that will never exist.

I totally understand there are problems with allowing you to import from the .ts file instead; again, thanks so much for the useful links in this thread! I think I'm going to stick with recommending --moduleResolution node and keeping my typesVersions setup for the time being, but definitely looking forward to seeing how this evolves moving forward.

@andrewbranch
Copy link
Member

@lemonmade I think you may have lost me between what configuration you are using for the development of Quilt and what configuration Quilt is generating for users. E.g., you said

This tool produces no .js files

but the exports config mentions JS files. I also see in your project’s root tsconfig.json you have an outDir set, so I’m not quite following who has no .js files. Seems like everyone does 😄

@dummdidumm
Copy link

Maybe we should pick that into node?

  • Seems like a "duh!" but then...why don't we see lots of demand for this!?

Consider this a voice of demand then 😄 In SvelteKit we have to keep a somewhat ugly ambient modules hack because of this (similar to @Josh-Cena) (https://github.com/sveltejs/kit/blob/master/packages/kit/types/ambient.d.ts#L71) and we'd very much like to switch to using export maps only instead, which right now doesn't work because the export resolution is not part of the node resolution algorithm. Since Node 12.X export maps present means they are used regardless of whether or not you use the commonjs or esm resolution, so I feel like a backport makes sense. But I also understand that maybe some people with older Node versions don't want this behavior, so we need an additional node10 setting now which preserves the old behavior? I certainly don't envy you 😅

It's also very strange that we need to reference ambient files (d.ts files) like import "./ambient.js" which feels wrong (is this related to the "lie" statement in the meeting notes?)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Notes Notes from our design meetings
Projects
None yet
Development

No branches or pull requests

7 participants