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

Implement TypeScript resolver plugin #4936

Closed
devongovett opened this issue Jul 26, 2020 · 35 comments · Fixed by #8807
Closed

Implement TypeScript resolver plugin #4936

devongovett opened this issue Jul 26, 2020 · 35 comments · Fixed by #8807

Comments

@devongovett
Copy link
Member

TypeScript supports some additional features in its resolver on top of the Node resolution algorithm. These are configured in tsconfig.json.

  • baseUrl - a root directory from which to resolve non-relative paths, in addition to node_modules. See also: [TypeScript] baseUrl inside tsconfig.json not supporting. #202.
  • paths - additional directories, relative to baseUrl, that should also be searched.
  • rootDirs - virtual directories that relative paths can be resolved from

There's also the legacy module resolution mode that TS supports. Not sure if that's something we should also support, but I guess someone might still use it. 🤷

Implementation

These resolution features should be supported in Parcel for imports declared in .ts or .tsx files. This can be done via a new resolver plugin, included in the default config: @parcel/resolver-typescript. This resolver should look at the sourcePath property of the dependency, and if it has a .ts or .tsx extension, it should resolve using the TS compiler API's resolveModuleName method. It should use a custom ModuleResolutionHost that ensures the Parcel file system is used instead of the Node one. This can be created using the @parcel/ts-utils package, which already includes an FSHost package implementing much of the required interface.

If a module is not resolved, or the source file is not a .ts or .tsx file, then the resolver should return null. This will then fall back to the default Parcel resolver, which will handle Parcel specific features like aliases.

Open questions

  • We'll need to determine whether we can properly track all of the files that the TS resolver reads so we can invalidate the Parcel cache properly. This is a work in progress for the default resolver as well.
  • It's unclear how the TS resolver should interact with Parcel features like aliases and named pipelines (e.g. url:./foo.jpg to import an image as a url). Theoretically we could fall back to the default resolver if TS doesn't find anything, but what if you alias react to preact but react is found by the TS resolver?
@101arrowz
Copy link
Member

101arrowz commented Jul 26, 2020

In response to the open questions:

  1. From the TS compiler API docs, it's possible to use a custom ModuleResolutionHost that implements fileExists and readFile. With this, it should be possible to track every file read by resolveModuleName.
  2. IDEs will complain about unknown import syntax when using import url from 'url:./foo.jpg', and there is nothing Parcel can do to change that. Therefore, I would think that no Parcel users would use a Parcel-specific import directly from TS. Parcel imports from TS projects will typically either use NodeJS require, or be imported from a JS file that can then be imported by TS (with an accompanying .d.ts). So Parcel should resolve requires from TS and let the TS resolver resolve imports. That is, if it is possible to differentiate require and import.

@devongovett
Copy link
Member Author

Importing images, CSS, etc. from TS-based UI components seems like it‘s fairly common, and should be supported by Parcel. Also other languages that TS doesn’t natively understand like .vue and .svelte. I believe it’s possible to build TS language service extensions to handle these eg https://github.com/mrmckeb/typescript-plugin-css-modules

@DeMoorJasper
Copy link
Member

DeMoorJasper commented Jul 26, 2020

@101arrowz the syntax issue is fairly simple to solve just configure a declaration file with something like this (like Devon already mentioned this is fairly common, I used to do it all the time for svg to react transformers):

declare module 'url:*' {
  const url: string;
  export default url;
}

and it will no longer throw syntax errors, as this is a standard Parcel feature it's probably a good idea to write a config/plugin for people in the docs that really supports everything as this declaration file trick I just mentioned does break the path autocomplete.

@101arrowz
Copy link
Member

I was not aware of the existence of TS language service plugins, which would likely be the best solution for custom Parcel imports. I'd be willing to help with the implementation.

@mischnic
Copy link
Member

I see a fair amount of quick solutions here, but I spent a lot of time on my tsconfig and just do not want to switch away. It kind of irked me that there wasn't a good path forward and seems like there won't be, so I created parcel-resolver-tspaths that reads your tsconfig.json for aliases. V2 users can find it on npm. I'd love feedback or to hear if this helps anyone. In the meantime I'll finally continue to use my @components/, @styles/, etc ;)

Originally posted by @zachbryant in #202 (comment)

@jacekkopecky
Copy link

it looks like parcel-resolver-tsc-path by @extraarya is a step towards this, but it seems that parcel API has changed and where the resolver expects filePath it now gets specifier.

Changing

  async resolve({ filePath, dependency, options, logger }) {

to

  async resolve({ specifier: filePath, dependency, options, logger }) {

fixes that. I can't find a public repo for parcel-resolver-tsc-path to submit a PR so hopefully @extraarya picks it up from here.

@bycaldr
Copy link

bycaldr commented Feb 23, 2022

@jacekkopecky

Public repo of parcel-resolver-tsc-path is here https://github.com/zachbryant/parcel-resolver-tspaths.

Change of filePath to specifier is not the only change that happened in Parcel. After fixing that, I had a problem with absolute/relative path resolution. I've created fixing PR and I'm using my fork in my project via. npm install https://github.com/bycaldr/parcel-resolver-tspaths.git until Parcel itself will cover this feature. Not the best solution, but it seems to work.

@jacekkopecky
Copy link

thanks @bycaldr ! I'll have a look when I get back to this.

@willstott101
Copy link

willstott101 commented Apr 6, 2022

I think it's worth also noting that typescript allows me to import with a .js extension - even if it's a .ts file. This means when I'm writing a library tsc needs to do pretty much nothing except strip types and re-emit with .js extensions. NodeJS will quite happily load the resulting code then with the .js extensions.

Form what I can tell this is becoming a popular setup as it brings the code you write in typescript closer to what would work natively in an a JS engine - where file extensions in imports is encouraged AFAICT.

Is that within scope of this issue?

edit:

FileA.ts

import { B } from "./FileB.js";

FileB.ts

export const B: string = "hello from the other side";

@zachbryant
Copy link

zachbryant commented Apr 19, 2022

@bycaldr and @ thread Updated the plugin after a bit of a hiatus. Should be all good now.

@jacekkopecky
Copy link

jacekkopecky commented Apr 27, 2022

I just tried with parcel-resolver-tspaths 0.0.7 and the paths in tsconfig.json

    "baseUrl": "./",
    "paths": {
      "@shared/*": ["pages/shared/*"]
    }

are still not taken into account for typescript import like this:

import Icon from '@shared/components/Icon';

The error I get from parcel build is

@parcel/core: Failed to resolve '@shared/components/Icon' from './pages/admin-edit-cohort/components/CohortLoader.tsx'

  …/pages/admin-edit-cohort/components/CohortLoader.tsx:9:18
     8 | 
  >  9 | import Icon from '@shared/components/Icon';

Was this actually supposed to be fixed by the recent updates @zachbryant ?

@zachbryant
Copy link

zachbryant commented Apr 27, 2022

@jacekkopecky could you make an issue in the repo, and also include your project folder structure? The latest update was to address some api changes in the 2.0 release

@zachbryant
Copy link

zachbryant commented May 7, 2022

I just noticed that the docs on module resolution mention using tilde or alias mappings, which I hadn't noticed the latter before. However, the latest parcel still cannot use tsconfig.json aliases. Is parcel specifically only allowing tilde resolution? Is it important to use baseUrl '.' for some reason, like the example? Would really appreciate clarity on this in the docs, that other aliases are not supported, because current phrasing pretty much says the opposite.

@fregante
Copy link
Contributor

fregante commented Jun 6, 2022

With TypeScript 4.7’s new moduleResolution: node16 it's become untenable to not have this in Parcel. The node16 setting requires using the .js extension for all imports just like Node 16 requires file extensions in ES modules.

To be clear moduleResolution: node enables using node_modules so it's not meant to actually produce code for Node. My current solution is to have a large configuration that includes every file of the project in package.json’s alias field:

{
  "alias": {
    "./this-stuff-is-just-for-local-parcel-tests": "https://github.com/parcel-bundler/parcel/issues/4936",
    "./source/sender.js": "./source/sender.ts",
    "./source/receiver.js": "./source/receiver.ts",
    "./source/types.js": "./source/types.ts",
    "./source/shared.js": "./source/shared.ts",
    "./source/api.js": "./source/api.ts",
    "./source/index.js": "./source/index.ts",
    "./source/thisTarget.js": "./source/thisTarget.ts",
    ETC
  }
}

Edit:

This is equivalent and works for all:

{
  "alias": {
    "./this-stuff-is-just-for-local-parcel-tests": "https://github.com/parcel-bundler/parcel/issues/4936",
    "./source/**/*.js": "./source/$1/$2.ts"
  }
}

@devongovett
Copy link
Member Author

Wait so you write .js even though you actually want .ts? That's horrible!

@fregante
Copy link
Contributor

fregante commented Jun 6, 2022

That's right. Tell the TypeScript team. The reasoning is that they don't want to rewrite the paths, so you have to write the final path and it'll ignore the extension.

@jakubmazanec
Copy link

See latest attempts to convince them here: microsoft/TypeScript#49083

@101arrowz
Copy link
Member

An alternative to creating a Parcel resolver for TypeScript syntax is to create a TypeScript resolver for Parcel syntax, that way you can use any Parcel import with "module": "node16". We would implement this in a similar fashion to @parcel/register but would probably also provide configuration to generate .dts declaration files for non-standard import syntax. I would personally prefer this strategy because it would also make url: imports and others also recognized by TypeScript and IntelliSense automatically, plus we wouldn't have to deal with TypeScript's ridiculous behavior of importing the destination .js file rather than the source file.

@devongovett
Copy link
Member Author

I replied on that issue. The official response seems to be that the node16 mode should only be used if your only target is running directly in Node, and you're compiling files 1:1. It's not recommended when you're using a bundler like Parcel. They might have a new mode in the future that allows .ts in import paths, but for now the recommendation is to use the old node mode without extensions. Does this work for you, or did you have a good reason for using node16 with Parcel?

@fregante
Copy link
Contributor

fregante commented Jun 11, 2022

Nothing is "only for node." The moduleResolution setting does add support for "node-only" imports from node_modules but bundlers exist specifically to bring node modules to the browser.

The node16 setting exists to align tsc's behavior with node, which tries to align its behavior with the browser (hence the requirement for extensions).

As for my case specifically:

  • my module is for the browser and I'm not distributing it bundled
  • I'm testing it in the browser, bundling via parcel locally
  • for it to be a proper ESM module, it should use the .js extension

The last point specifically is why many of us are here. Extensions are only enforced by this new setting, but they've been there for years in TypeScript.


As for this issue, if TypeScript can resolve it, then a "typescript resolver" presumably should too.

@caio-oliv
Copy link

I wanted to migrate my projects using webpack to parcel, but the lack of path resolution with baseUrl and paths was a roadblock to me.

Then I created a resolver (didn't knew that some already existed) and its working in some of my projects.

It needs more tests, but have some documentation, maybe someone could benefit from it.

parcel-resolver-typescript-module

@devongovett
Copy link
Member Author

I'm currently trying to implement support for baseUrl, paths, moduleSuffixes, and extension replacement (e.g. ./foo.js -> ./foo.ts) as part of a new resolver in Rust.

One question I've come across is how to load tsconfig.json files. TSC only loads a single tsconfig for the whole project from the CWD of the tsc CLI. Nested tsconfig.json files are ignored. However, in a monorepo, you might have multiple projects, each with their own tsconfig that you would normally build by running tsc for each one. But with Parcel, we might be building all of these at once. We could search upward from each file to find the nearest tsconfig.json, but this would technically not match TSC exactly. That's what esbuild seems to do though. I'm not sure there is really a way to know where tsc would be run from otherwise. Does anyone know how other tools handle this?

@niklas-e
Copy link

@devongovett not sure how to do it in Parcel as I'm not enough familiar with it. But as you said in a monorepo you'd usually run tsc inside one of the project directories, which causes it to read the tsconfig inside the project directory. Therefore to have common configurations for whole repo would be to have tsconfig inside the root of the repo, which would be used as a base inside the project specific tsconfigs by using extends property. Thus if Parcel knows which directories are project root directories it would probably help in determining which config to use.

At least that is what I've seen and done myself. I'd guess there would be some design flaw in project structure if you'd have additional nested configurations in addition to base+project 🤔

@devongovett
Copy link
Member Author

Yeah I decided to search upward for the nearest tsconfig.json file from the source file containing the import, and resolve using that. That will be the closest without requiring manual configuration of all of the project roots. This will not apply within node_modules and will only apply for dependencies in JS and TS files (e.g. paths will not apply to CSS files).

@Utsav2
Copy link

Utsav2 commented Feb 8, 2023

I'm curious if this should also apply to project references - the project reference seems very similar to the paths variable.

@devongovett
Copy link
Member Author

#8807 adds support for the following tsconfig features:

  • paths
  • baseUrl
  • moduleSuffixes

It also matches TSC's behavior in regards to extensions: Importing a file with a .js extension will resolve to a .ts or .tsx file, a .mjs extension will resolve to a .mts file, and .cjs will resolve to .cts. Importing using a .ts or .tsx extension explicitly also continues to be supported, in addition to omitting the extension.

This is now released in nightly if anyone wants to try it and report back. Thanks!

@andrewbranch
Copy link

Hi @devongovett, I’m testing how various bundlers handle imports from .mjs and .mts files, and I took your previous message to imply that Parcel has support for .mts files in nightly now, but I get the message No transformers found for __src/0/index.mts__ in 2.0.0-nightly.1314. Is this expected? My config is

let bundler = new Parcel({
  entries: path.resolve(__dirname, `../../src/${filename}`),
  defaultConfig: require.resolve("@parcel/config-default"),
  mode: "production",
  targets: {
    main: {
      engines: {
        node: "14",
      },
      distDir: path.resolve(__dirname, `../../dist/${idx}`),
    },
  },
});

and the package.json next to the entry is {}. Thanks!

@devongovett
Copy link
Member Author

I guess we'd need to add mts and cts here:

"*.{js,mjs,jsm,jsx,es6,cjs,ts,tsx}": [

@andrewbranch
Copy link

Is there a way to easily override that default config value in the options I pass in?

@devongovett
Copy link
Member Author

Yes. You can create a .parcelrc in your project with something like this:

{
  "extends": "@parcel/config-default",
  "transformers": {
    "*.{mts,cts}": [
      "@parcel/transformer-babel",
      "@parcel/transformer-js",
      "@parcel/transformer-react-refresh-wrap"
    ]
  }
}

@andrewbranch
Copy link

Is there a way to do that from the API without adding another file? (I’m programmatically running many bundlers for https://andrewbranch.github.io/interop-test, so adding file-based config is a bit of a pain.)

@andrewbranch
Copy link

andrewbranch commented May 24, 2023

I went ahead and added a config file, and FYI the bundled output crashes in Node with stuff like

Command failed: node dist/6/index.js
node:internal/modules/cjs/loader:1073
  throw err;
  ^

Error: Cannot find module '55865b4f7fd0dd89'
Require stack:
- /Users/andrew/Developer/microsoft/interop-test/dist/6/index.js
    at Module._resolveFilename (node:internal/modules/cjs/loader:1070:15)
    at Module._load (node:internal/modules/cjs/loader:923:27)
    at Module.require (node:internal/modules/cjs/loader:1137:19)
    at newRequire (/Users/andrew/Developer/microsoft/interop-test/dist/6/index.js:58:18)
    at newRequire (/Users/andrew/Developer/microsoft/interop-test/dist/6/index.js:45:18)
    at localRequire (/Users/andrew/Developer/microsoft/interop-test/dist/6/index.js:84:35)
    at 9fdGS.util (/Users/andrew/Developer/microsoft/interop-test/dist/6/index.js:151:1)
    at newRequire (/Users/andrew/Developer/microsoft/interop-test/dist/6/index.js:71:24)
    at /Users/andrew/Developer/microsoft/interop-test/dist/6/index.js:122:5
    at Object.<anonymous> (/Users/andrew/Developer/microsoft/interop-test/dist/6/index.js:145:3) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [ '/Users/andrew/Developer/microsoft/interop-test/dist/6/index.js' ]
}

So I guess there may be a bit more to do to support .mts files.


Edit: the resolution errors seem to occur only in test cases that use dynamic import. The results are now published at https://andrewbranch.github.io/interop-test/#parcel-mts, and you can see the methodology at https://github.com/andrewbranch/interop-test/blob/main/lib/runners/runParcelTest.js if you’re interested in obtaining a repro.

@mischnic
Copy link
Member

Looks like you've run into what's described in #8468: with your config, the current semantics of parcelrc mean that @parcel/transformer-js is executed twice which then causes some dependency ids to mismatch, leading to that error.
You can do this as a workaround:

--- lib/runners/runParcelTest.js
+++ lib/runners/runParcelTest.js
@@ -26,7 +26,7 @@ module.exports = async (runtime, variant, ext, packageJsonType) => {
             const parcelrcContent = {
               extends: "@parcel/config-default",
               transformers: {
-                "*.mts": [
+                "*.{js,mjs,jsm,jsx,es6,cjs,ts,tsx,mts,cts}": [
                   "@parcel/transformer-babel",
                   "@parcel/transformer-js",
                 ]

@andrewbranch
Copy link

Excellent, that does fix it. Thanks!

@Darrenbydesign
Copy link

So what is the resolution to this? Is there any update to the Parcel Documentation site about using this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.