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

Add "local" option to "JSX" to support VueJS TSX files #12067

Closed
notoriousb1t opened this issue Nov 5, 2016 · 7 comments
Closed

Add "local" option to "JSX" to support VueJS TSX files #12067

notoriousb1t opened this issue Nov 5, 2016 · 7 comments
Labels
Fixed A PR has been merged for this issue Suggestion An idea for TypeScript

Comments

@notoriousb1t
Copy link

TypeScript Version: 2.0.3 / nightly (2.1.0-dev.201xxxxx)

I think we should add another type to the "jsx" option in tsconfig.json named "local". This would expect a "createElement" function to be in scope when rendering .tsx files.

I think this mode would allow VueJS + TSX to work better. Currently, the "preserve" option needs to be set and then another transformer (there is a babel transformer) needs to be used on the output file. I think the integration would be a lot simpler with that change and wouldn't require a second transpiler.

I also think this change would make it easier for future libraries since they won't have to register globals, but instead simply supply a function that matches the react/vue createElement syntax.

Here is how I expect it to work:

The following would compile properly

TSX file

import { style } from 'typestyle';
import { HelloComponent } from './components'

const appStyles = style({
    fontFamily: 'Avenir, Helvetica, Arial, sans-serif',
    '-webkitFontSmoothing': 'antialiased',
    '-mozOsxFontSmoothing': 'grayscale',
    textAlign: 'center',
    color: '#2c3e50',
    marginTop: 60,
});

export class AppComponent {
    name = 'app';
    components = {
        Hello: HelloComponent
    };
    render(createElement) {
        return (<div id="app" className={appStyles}>
                <img src="./assets/logo.png" />
                <hello></hello>
        </div>);
    }
}

Output file

"use strict";
const typestyle_1 = require("typestyle");
const components_1 = require("./components");
const appStyles = typestyle_1.style({
    fontFamily: 'Avenir, Helvetica, Arial, sans-serif',
    '-webkitFontSmoothing': 'antialiased',
    '-mozOsxFontSmoothing': 'grayscale',
    textAlign: 'center',
    color: '#2c3e50',
    marginTop: 60,
});
class AppComponent {
    constructor() {
        this.name = 'app';
        this.components = {
            Hello: components_1.HelloComponent
        };
    }
    render(createElement) {
        return (createElement("div", { id: "app", className: appStyles },
            createElement("img", { src: "./assets/logo.png" }),
            createElement("hello", null)));
    }
}
exports.AppComponent = AppComponent;

And the following change (remove createElement from arguments) would cause a compile error:

    render() {
        return (<div id="app" className={appStyles}>
                <img src="./assets/logo.png" />
                <hello></hello>
        </div>);
    }

Error message:

Christophers-MacBook-Pro:just-motion2 cwallis$ node ../typescript/built/local/tsc
src/App.tsx(19,18): error TS2304: Cannot find name 'createElement'.
src/App.tsx(20,18): error TS2304: Cannot find name 'createElement'.
src/App.tsx(21,18): error TS2304: Cannot find name 'createElement'.

I realize I have done this out of order. I decided to figure out how it could be done, and then saw the checklist while trying to put in a pull request. I have a commit to a local fork that has a working version of this: notoriousb1t@44ade7a. I have signed the CTA. I need to add some unit tests to that, but I am otherwise ready to submit a PR if this addition will be accepted.

@DanielRosenwasser DanielRosenwasser added Suggestion An idea for TypeScript In Discussion Not yet reached consensus labels Nov 6, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Nov 7, 2016

how is different from #9582: --jsxfactory createElement ?

@notoriousb1t
Copy link
Author

notoriousb1t commented Nov 7, 2016

@mhegazy, I was not aware of that issue when I forked the code and then created this issue. (and honestly with that title, I am not surprised I didn't see it among the other 1000 issues when I did a search)

Taking a look through that thread, it looks like it is trying to solve the problem by modifying the existing behavior of the jsx: + 'react' and reactNamespace options to support a custom symbol name for createElement.

My proposal creates a new mode for this and does not modify those other configuration options. I made a point to make as few changes as possible in implementing the feature in my fork of TypeScript. The new code simply checks that there is a createElement in scope and adds it. I think that having to write render(createElement) instead of render(h) is reasonable.

Honestly though, the other issue if put into the prod build of TypeScript would solve the same issue I am trying to solve (making it easier to work with VueJS and TSX). The other approach is more configurable for better or worse. The approach listed here should be safer in theory.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 7, 2016

Assuming that #9582 covers your needs, i would say it provides a better way than adding a new "mode". so do you agree that #9582 is sufficient for this scenario?

@notoriousb1t
Copy link
Author

Assuming its PR ever gets accepted. #11267 (review) hasn't been touched in nearly a month, so I am a bit skeptical in thinking that this issue is done and solved.

I personally think it is better to reduce complexity where-ever possible, even in things like configuration options in libraries. However, either approach would suit my needs when accepted 😄

@sheetalkamat sheetalkamat added the Fixed A PR has been merged for this issue label Nov 10, 2016
@notoriousb1t
Copy link
Author

Thank you @sheetalkamat! 😄

@mhegazy mhegazy added this to the TypeScript 2.1.3 milestone Nov 10, 2016
@mhegazy mhegazy removed the In Discussion Not yet reached consensus label Nov 10, 2016
@kristianmandrup
Copy link

Is it jsxFactory or reactFactory as of 2018?? I assume it can be used as compiler option either in CLI or in tsconfig.json

@mhegazy
Copy link
Contributor

mhegazy commented Feb 13, 2018

jsxFactory is the recommended setting.

@microsoft microsoft locked and limited conversation to collaborators Jul 3, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Fixed A PR has been merged for this issue Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests

5 participants