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

How would one transform emitted .d.ts files? #1222

Closed
resynth1943 opened this issue Dec 12, 2020 · 19 comments
Closed

How would one transform emitted .d.ts files? #1222

resynth1943 opened this issue Dec 12, 2020 · 19 comments
Labels

Comments

@resynth1943
Copy link

resynth1943 commented Dec 12, 2020

Expected Behaviour

The emitted .d.ts files go through the transformers listed within getCustomTransformers.

Actual Behaviour

They don't.

Steps to Reproduce the Problem

  • Use ts-loader to compile a TypeScript project.
  • Set your tsconfig to emit declaration files.
  • Add a transformer within ts-loader's getCustomTransformers option.
  • The .d.ts files don't go through the transformer.

Location of a Minimal Repository that Demonstrates the Issue.

Don't have one right now. If this is requested, I'll make one.


Is this intended behaviour?
Would appreciate any help you can give me if it's not.

@johnnyreilly
Copy link
Member

It's the first time I've heard of this use case.... Does tsc support this directly?

@resynth1943
Copy link
Author

Yeah, it must do. If you check the tsc.SourceFile declaration, there's a member named isDeclarationFile.

@resynth1943
Copy link
Author

Just to expand a little on the use-case: I have a code-base which supports both Deno and Node.

Deno requires import paths to end in .ts, which has caused numerous people to ask the TypeScript maintainers to allow said functionality.

So, to meet the needs of both platforms, my Deno code lies in ./src, while my Node.js code lies in ./lib.
As TypeScript does not modify import paths, I have created a transformer to do just that.
Currently, the transformer does not run on .d.ts files, which means I end up with import [...] from './[...].ts code in my declaration files (breaking type-checking for ./lib).

So, all in all, I'd say this is a perfectly valid use-case :-)

@johnnyreilly
Copy link
Member

Interesting! Where does webpack fit into this?

@resynth1943
Copy link
Author

I use Webpack to transpile the TypeScript (I have x.min.js and x.js bundles), plus ts-loader doesn't fail if an import path ends with .ts (thank you so much!).

@johnnyreilly
Copy link
Member

So this is a web app then? Using webpack implies node. How does Deno fit in?

@resynth1943
Copy link
Author

resynth1943 commented Dec 13, 2020

It's not a web-app, it's a library.

Deno does not need TypeScript compilation, so you directly import the source code of the library as a URL.
Whereas Node can not read TypeScript on its own, so it needs the compiled JavaScript in ./lib (alongside typings for Intellisense).

@johnnyreilly
Copy link
Member

Very interesting - so you're using webpack but you're not building a web app?

Sorry I'm a little puzzled... I'm not quite following why you aren't directly using tsc or Deno... wouldn't it be easier?

@resynth1943
Copy link
Author

resynth1943 commented Dec 13, 2020

You don't need to use Webpack for web apps.

I am using Webpack because:

Deno requires import paths to end in .ts, which has caused numerous people to ask the TypeScript maintainers to allow said functionality.

plus ts-loader doesn't fail if an import path ends with .ts

microsoft/TypeScript#38149

Anyway, are we any closer to fixing the issue?

@johnnyreilly
Copy link
Member

Anyway, are we any closer to fixing the issue?

I appreciate it's an issue for you. However, this does seem to be a fairly niche use case. That's not to say it's not worth catering for. If you'd like to start work on a prospective PR I'll certainly take a look.

Don't worry about tests initially. Let's see where this goes.

@lukescott
Copy link

I'm not sure if this is related, but I pretty much had to drop ts-loader and use tsc directly because of a .d.ts issue. Basically we have some legacy source in a referenced TypeScript package. Because we sometimes update this file from a different js project, we keep it as regular JavaScript with the allowTs on. We added a same-named .d.ts file so we could express the types on the boundary. This works in tsc, but fails in ts-loader, which acts like the type declaration file does not exist.

@resynth1943
Copy link
Author

Anyway, are we any closer to fixing the issue?

I appreciate it's an issue for you. However, this does seem to be a fairly niche use case.

Ehh. After some thought, I'm going to have to disagree.

This is the cleanest solution I can find, that allows Deno and Node source code in the same repository, instead of maintaining separate branches. I need to support both Deno and Node, as I use this library in both those environments.

Simply put, this is somewhat of an X/Y solution.

I can appreciate that Deno's style of imports is somewhat unconventional, but it should definitely be catered for.

As for the PR: I don't have any experience with this code-base, but I can give it a go.

@johnnyreilly
Copy link
Member

Have a go!

@neil-ji

This comment has been minimized.

@resynth1943

This comment has been minimized.

@appzuka
Copy link
Member

appzuka commented Dec 29, 2020

The ts-loader readme gives the following type signature for getCustomTransformers:

(program: Program) => {
  before?: TransformerFactory<SourceFile>[];
  after?: TransformerFactory<SourceFile>[];
}

From a little Googling I found there is a third option: afterDeclarations. If you want to transform emitted d.ts files (which I understand is the aim here) it is afterDeclarations you need to use. I created a small repository based on the CustomTransformers test case. The test only covers the 'before' case, which is probably the most common case to use. I added a transformer for afterDeclarations. The transformation it applies is pointless but it proves that it works as expected. You can see the repository here:

https://github.com/appzuka/ts-loader-afterDeclarations-transformer.git

The following repository shows how to transform imports in d.ts files, which may be close to your requirement:

https://github.com/longlho/ts-transform-json

If you are already using afterDeclarations and it is not working for you perhaps you could publish a repository with a minimal test case showing what is not working.

@drewdelano
Copy link

@resynth1943 did that fix the issue for you?

@stale
Copy link

stale bot commented Apr 17, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the wontfix label Apr 17, 2022
@stale
Copy link

stale bot commented Apr 28, 2022

Closing as stale. Please reopen if you'd like to work on this further.

@stale stale bot closed this as completed Apr 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants