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

Stopped working after upgrade to 2.0.0 #72

Closed
tareqdayya opened this issue Sep 25, 2020 · 17 comments · Fixed by #76
Closed

Stopped working after upgrade to 2.0.0 #72

tareqdayya opened this issue Sep 25, 2020 · 17 comments · Fixed by #76

Comments

@tareqdayya
Copy link

tareqdayya commented Sep 25, 2020

Hi, this plugin stopped working after upgrading from 1.1.14 to 2.0.1.
Here's my config.

{
  "compilerOptions": {
    "target": "es5",
    "module": "commonjs",
    "lib": [
      "ES2020",
      "ESNext",
      "ES7",
      "ES6"
    ],
    "outDir": "./prod",
    "skipLibCheck": true,
    "noImplicitAny": true,
    "baseUrl": "./src",
    "esModuleInterop": true,
    "forceConsistentCasingInFileNames": true,
    "plugins": [
      {
        "transform": "typescript-transform-paths"
      }
    ]
  },
  "include": [
    "src"
  ]
}

I use it with ttsc (ttypescript).

@nonara
Copy link
Collaborator

nonara commented Sep 25, 2020

Hi Tareq,

We'll probably need more information to be of any help. Is there an error? What is the behaviour that's happening and what were you expecting?

I don't see a paths property in your config. The plugin isn't going to do anything unless you have paths configured.

@tareqdayya
Copy link
Author

Hi @nonara , Thanks for answering.
There's no error, it just doesn't do what it's supposed to (doesn't transform paths.)
Yup, I don't have a paths property. It seems the 1.1.14 version works without one.
Anyway, I downgraded and it seems to work without issues.
Thanks,

@danielpza
Copy link
Member

@tareqdayya what typescript version are you using?

@tareqdayya
Copy link
Author

Hi @danielpza
I've tried it with 3.7.2 and 4.0.3. Results were identical.

@nonara
Copy link
Collaborator

nonara commented Sep 25, 2020

@tareqdayya Hmm. Would you mind providing an example of the before and after on one of the transforms without paths configured in v1.1.14?

@nonara
Copy link
Collaborator

nonara commented Sep 25, 2020

Ok. I had a look at the old code, and I think I see what's going on.

@tareqdayya Try adding this to your tsconfig, and it should work with v2:

paths: {
  '*': [ '*' ]
}

We updated the logic to rely on TS Compiler API, which has improved many issues. It's also updated to support TS v4, which in a few versions will deprecate the old API, causing v1.x of this plugin to break. I would recommend upgrading to v2.

Still interested in seeing an example of what you were transforming. I'm not entirely sure I understand your use case of transforming without paths.

@tareqdayya
Copy link
Author

Ok. I had a look at the old code, and I think I see what's going on.

@tareqdayya Try adding this to your tsconfig, and it should work with v2:

paths: {
  '*': [ '*' ]
}

We updated the logic to rely on TS Compiler API, which has improved many issues. It's also updated to support TS v4, which in a few versions will deprecate the old API, causing v1.x of this plugin to break. I would recommend upgrading to v2.

Still interested in seeing an example of what you were transforming. I'm not entirely sure I understand your use case of transforming without paths.

@nonara Yup :) That fixes it.

What i'm doing is using absolute imports relative to baseUrl. For example:
import router from 'controllers';
Would import from src/controllers; src being my baseUrl in tsconfig. I would use the same import from any file.
I've upgraded to v2. Will monitor it for a while and let you know in case there's an issue with this approach.
Thanks,

@nonara
Copy link
Collaborator

nonara commented Sep 25, 2020

@tareqdayya Ok. Thanks! That makes sense. Glad it's all set!

@tareqdayya tareqdayya reopened this Sep 25, 2020
@tareqdayya
Copy link
Author

tareqdayya commented Sep 25, 2020

Ok, so this went sour pretty quickly 😂
I've run into this error with version 2 (i've tried downgrading, the problem doesn't occur in the older version):

/Users/mac/Desktop/venv/var/www/Playgrounds/linqpal-challenge/server/node_modules/typescript/lib/typescript.js:103736
                throw e;
                ^

TypeError: Cannot read property 'text' of undefined
    at getErrorSpanForNode (/Users/mac/Desktop/venv/var/www/Playgrounds/linqpal-challenge/server/node_modules/typescript/lib/typescript.js:13660:40)
    at createDiagnosticForNodeInSourceFile (/Users/mac/Desktop/venv/var/www/Playgrounds/linqpal-challenge/server/node_modules/typescript/lib/typescript.js:13564:20)
    at Object.createDiagnosticForNode (/Users/mac/Desktop/venv/var/www/Playgrounds/linqpal-challenge/server/node_modules/typescript/lib/typescript.js:13555:16)
    at error (/Users/mac/Desktop/venv/var/www/Playgrounds/linqpal-challenge/server/node_modules/typescript/lib/typescript.js:43205:22)
    at resolveExternalModule (/Users/mac/Desktop/venv/var/www/Playgrounds/linqpal-challenge/server/node_modules/typescript/lib/typescript.js:45212:25)
    at resolveExternalModuleNameWorker (/Users/mac/Desktop/venv/var/www/Playgrounds/linqpal-challenge/server/node_modules/typescript/lib/typescript.js:45129:19)
    at resolveExternalModuleName (/Users/mac/Desktop/venv/var/www/Playgrounds/linqpal-challenge/server/node_modules/typescript/lib/typescript.js:45124:20)
    at checkImportDeclaration (/Users/mac/Desktop/venv/var/www/Playgrounds/linqpal-challenge/server/node_modules/typescript/lib/typescript.js:74645:49)
    at checkSourceElementWorker (/Users/mac/Desktop/venv/var/www/Playgrounds/linqpal-challenge/server/node_modules/typescript/lib/typescript.js:75078:28)
    at checkSourceElement (/Users/mac/Desktop/venv/var/www/Playgrounds/linqpal-challenge/server/node_modules/typescript/lib/typescript.js:74911:17)
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.
error Command failed with exit code 1.
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.

You won't believe how this happens, brace yourselves. I've got this code:

import { UserType } from 'types/types';

interface UserModel extends UserType, Document {}

If we switch the order of UserType and Document, the error happens and the application crashes. How? Why? Where? When? I have no idea.
I thought about opening a new issue but the subject of the OP is kinda still relevant. Will open a new issue if you think it's better.

@nonara
Copy link
Collaborator

nonara commented Sep 25, 2020

Interesting. We can keep it in this issue. Can you see if you can create a minimal types.ts with a UserType that will reproduce it, so I can look into it?

@tareqdayya
Copy link
Author

@nonara Yeah absolutely. types.ts actually contains only one interface:
export interface UserType { firstName: string, lastName: string, telNumber: string, fullAddress: string, SSN: string, isAdmin: boolean, }
The Document type on the other hand is imported from mongoose.

@osyrisrblx
Copy link

osyrisrblx commented Sep 27, 2020

Seeing a similar issue. I have a .d.ts file that only exports types and another file that does:

export * from "path/to/types";

This seems to now compile instead of being erased.

Should be simple enough to reproduce, but let me know if you can't. I could put together a tiny repo to reproduce it.

@nonara
Copy link
Collaborator

nonara commented Sep 27, 2020

@osyrisrblx That's due to a change in TypeScript 3.9. See: Export * is always retained

Beginning with v2.0 of this plugin, we standardized behaviour to match TypeScript's.

@clintwood
Copy link

@nonara - FYI I'm getting the same or similar issue as @tareqdayya gets above when using ttsc --watch and make any change that triggers a recompilation. For example if I open any source file and save it (even without any code changes) to trigger a watch change I get the dump below.

I have confirmed that it is as a result of this plugin as ttsc does not dump without the plugin in the tsconfig.

Dump from any file change when using ttsc --watch:

[12:43:28] File change detected. Starting incremental compilation...

/Users/user/project/node_modules/typescript/lib/typescript.js:103732
                throw e;
                ^

TypeError: Cannot read property 'text' of undefined
    at getErrorSpanForNode (/Users/user/project/node_modules/typescript/lib/typescript.js:13653:40)
    at createDiagnosticForNodeInSourceFile (/Users/user/project/node_modules/typescript/lib/typescript.js:13557:20)
    at Object.createDiagnosticForNode (/Users/user/project/node_modules/typescript/lib/typescript.js:13548:16)
    at error (/Users/user/project/node_modules/typescript/lib/typescript.js:43198:22)
    at resolveExternalModule (/Users/user/project/node_modules/typescript/lib/typescript.js:45205:25)
    at resolveExternalModuleNameWorker (/Users/user/project/node_modules/typescript/lib/typescript.js:45122:19)
    at resolveExternalModuleName (/Users/user/project/node_modules/typescript/lib/typescript.js:45117:20)
    at checkImportDeclaration (/Users/user/project/node_modules/typescript/lib/typescript.js:74641:49)
    at checkSourceElementWorker (/Users/user/project/node_modules/typescript/lib/typescript.js:75074:28)
    at checkSourceElement (/Users/user/project/node_modules/typescript/lib/typescript.js:74907:17)

@nonara
Copy link
Collaborator

nonara commented Oct 7, 2020

Ok, so here's the update on this one for everyone.

This problem is linked to an issue in the TypeScript API that has been around for awhile. Namely, type emit elision breaks when we modify either ImportDeclaration or ExportDeclaration nodes (including any children). We were able to workaround this issue in TS 3 by using an internal API method called updateNode. Unfortunately, that method has been removed in TS4, meaning the best way I could fix it was to directly replace the string node for module specifier.

Unfortunately, this seems to cause an issue if there are any diagnostics attached to the former node, which makes sense.

This leaves us with the main options of:

  1. Hunting around to try to find another internal method in the API that might do the same this updateNode did
  2. Attempt to hotwire it to copy over the diagnostics.

Both of these would be time consuming and undesirable, in the sense that it's best not to use internal TS methods for a public project like this, as they're subject to breaking changes without notice.

Fortunately, however, the TS team added my reported issue to the milestone for TS 4.2.0

I'm assuming that means that it's in the roster to be fixed, but I noticed that it hasn't gotten a bug label yet. That in mind, I'll continue to track, and if they decide that it isn't an issue, I'll dig in and find a way to work around it on our end.

In the mean time, if you're experiencing this issue, I'd recommend using the latest available v1 of typescript-transform-paths and following this issue. When TS releases the fix, we'll create a new minor version of this package which removes the workaround, and I'll post a message here letting everyone know that it's all set!

Thanks everyone for sharing your reports!

@nonara
Copy link
Collaborator

nonara commented Nov 17, 2020

Hi all. This should be sorted now! I was able to steal some time to dig into the compiler a bit more.

Post-mortem is here if anyone is interested: #76

The short version is, you won't have to wait on a new TS version. It should be compatible with all TS versions. Anyone who was experiencing issues, I'd greatly appreciate it if you let me know if this fixes it for you!

@nonara
Copy link
Collaborator

nonara commented Nov 27, 2020

Just a heads up - I recommend upgrading to the latest v2.1.0. We implemented proper type elision. No more workarounds.

If interested, more detail can be found in #81

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

Successfully merging a pull request may close this issue.

5 participants