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

Usage question: Adding new import declarations #13

Open
LukasMachetanz opened this issue May 27, 2020 · 6 comments
Open

Usage question: Adding new import declarations #13

LukasMachetanz opened this issue May 27, 2020 · 6 comments

Comments

@LukasMachetanz
Copy link

LukasMachetanz commented May 27, 2020

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

@itsdouges
Copy link
Owner

itsdouges commented May 27, 2020

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

@LukasMachetanz
Copy link
Author

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.

@itsdouges
Copy link
Owner

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?

@LukasMachetanz
Copy link
Author

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
Copy link
Author

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. ;)

@itsdouges
Copy link
Owner

I've gotten this reply from the TS team as to how we could work around the problem microsoft/TypeScript#38077

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

No branches or pull requests

2 participants