typescript-transformer-handbook icon indicating copy to clipboard operation
typescript-transformer-handbook copied to clipboard

Usage question: Adding new import declarations

Open LukasMachetanz opened this issue 4 years ago • 6 comments

Hey!

Once again I would like first of all to say that I really appreciate this repository and the provided examples. E.g. the example regarding Adding new import declarations.

Although I have a question regarding this topic. Let's assume I have the following class:

export class TestComponent {
    public greet(): string {
        return "Hello";
    }
}

The builded file with my custom TypeScript Transformer in place looks like this:

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var TestComponent = /** @class */ (function () {
    function TestComponent() {
    }
    TestComponent.prototype.greet = function () {
        return jasmine.createSpy().and.callFake(function () {
            if (!Ttransformer.isInTransformContext()) {
                return "Hello";
            }
        });
    };
    return TestComponent;
}());
exports.TestComponent = TestComponent;
//# sourceMappingURL=test.component.js.map

I would like to add an import statement to provide the Ttransformer class. Therefore I adjusted my transformer to do so. The output looks now like this:

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });

// === THE FOLLOWING LINE WAS ADDED NOW ===
var ttransformer_1 = require("../ttransformer"); 
 
var TestComponent = /** @class */ (function () {
    function TestComponent() {
    }
    TestComponent.prototype.greet = function () {
        return jasmine.createSpy().and.callFake(function () {
            if (!Ttransformer.isInTransformContext()) {
                return "Hello";
            }
        });
    };
    return TestComponent;
}());
exports.TestComponent = TestComponent;
//# sourceMappingURL=test.component.js.map

I am really wondering if the resulting source code is correct. If I already import the class in the first place, like in this example...

import { Ttransformer } from "../ttransformer";
Ttransformer.isInTransformContext();

...the ouptut looks like this:

"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var ttransformer_1 = require("../ttransformer");
ttransformer_1.Ttransformer.isInTransformContext();
//# sourceMappingURL=source.dev.js.map

The actual difference is shown here:

  1. Ttransformer.isInTransformContext() vs.
  2. ttransformer_1.Ttransformer.isInTransformContext();

Do I have to make sure in my custom transformer that ttransformer_1 get's added? I would have imagined that this happens automatically. As I said; I really like the provided example; but I am wondering how it would look like if you would try to use the actual import. Do you know what I mean?

I appreciate any thoughts, ideas and explanation. Thanks in advance.

Greets, Lukas

LukasMachetanz avatar May 27 '20 10:05 LukasMachetanz

heya! currently there are issues (either a bug, oversight, or deliberate thing by the TypeScript team) when compiling down to common js and applying custom transformers - basically the imports don't get "bound" in the binding step and thus their usage areas don't get renamed...

i got around this in Compiled by... hacks... I'm not sure if this is in the handbook TBH https://github.com/atlassian-labs/compiled-css-in-js/blob/master/packages/ts-transform/src/constants.tsx#L48

itsdouges avatar May 27 '20 10:05 itsdouges

Hey. Thank you for the immediate answer. Do I understand it correctly that the provided link is already the solution to the described problem? If this is the case I definitely have to try it. I am curious if it will work.

And btw why do you know such stuff? Unbelievable. :) Am I wrong or is there really not much documentation out there? Anyhow; great work. I really appreciate the support.

LukasMachetanz avatar May 27 '20 11:05 LukasMachetanz

It's a work around not really a solution 😅 - I found this by trial and error. There really isn't much information about this which is why I made the handbook in the first place.

If you get this working want to contribute back and mention this workaround in the handbook?

itsdouges avatar May 27 '20 11:05 itsdouges

Ah; I see. ;)

I will try it. Hopefully I get it to work without the need to consult you again. :) If I am successful I would be glad contributing back. As I said, I really appreciate the handbook. Therefore I imagine that it could help others as well.

LukasMachetanz avatar May 27 '20 11:05 LukasMachetanz

Sorry but I have to clarify the solution again. I think I understand the code snippet. But I do not understand in which "context" they are applying it.

  • The whole "mechanism" starts when the target is CommonJS
  • If this is the case they just add the "binding"
  • If not ... the "binding" does not have to be added

Is this right? So in short: They are doing what I had in mind: it is necessary to add the "binding" manually, right?

Just some questions if I got it correctly:

  • So for my example ttransformer_1 would have to be added manually, right?
  • The added import declaration is correct, right? It does not have to be touched?
  • In my case just the line if (!Ttransformer.isInTransformContext()) { would have to be updated, right?

This is the part from the transformer:

ts.createPropertyAccess(ts.createIdentifier("Ttransformer"), ts.createIdentifier("isInTransformContext")),

Here I would have to add the ttransformer_1 additionally, right?

So I would guess something like this:

  ts.createPropertyAccess(
    ts.createPropertyAccess(
      ts.createIdentifier("ttransformer_1"),
      ts.createIdentifier("Ttransformer")
    ),
    ts.createIdentifier("isInTransformContext")
  ),

I just wanted to clarify and check if I understood it correctly. In the best case you just have to give me a thumb up. Thanks. ;)

LukasMachetanz avatar May 27 '20 20:05 LukasMachetanz

I've gotten this reply from the TS team as to how we could work around the problem https://github.com/microsoft/TypeScript/issues/38077

itsdouges avatar Jun 30 '20 00:06 itsdouges