Potential language server changes about relay-compiler-language-typescript HOT 15 OPEN

I had indeed noticed that, but wasn’t too worried about it because at least it won’t show up when you’re typing. Nonetheless, completely hiding would be nicer indeed.

Actually property names with spaces do show up, just not when it is a leading space it seems :)

from relay-compiler-language-typescript.

Cheers! :)

I have asked around on that commit and it seems Relay will be allowing self referencing fragments in some forms in which it can be compiled/unrolled to a valid GraphQL Query. As such it seems the GraphQL spec is not changing in this aspect, but just that Relay will do its thing and make the query valid. We should probably look to incorporate those changes into this plugin as well.

from relay-compiler-language-typescript.

@alloy (forgot to tag you)

from relay-compiler-language-typescript.

I had indeed noticed that, but wasn’t too worried about it because at least it won’t show up when you’re typing. Nonetheless, completely hiding would be nicer indeed.

from relay-compiler-language-typescript.

Having tested this a bit, it seems we're still good for now (they auto complete for spaces and wonky characters, but not if they are leading, it's probably a bug). However we might want to look into using symbols for this.

Something like this:

File: react-relay.d.ts

export const Fragments$Symbol: unique Symbol;
export const FragmentId$Symbol: unique Symbol;

File __generated__/SomeFragment_data.graphql.ts

import { FragmentId$Symbol } from 'react-relay';

export declare const SomeFragment_data$ref: unique symbol;

export type SomeFragment_data = {
  [$FragmentId$Symbol]: typeof SomeFragment_data$ref;
  ...
}

As far as I can see this has all the same behaviors we currently get - only now we're not using fancy empty enums to achieve what the type system now handles quite well. I'll look into making a PR at some point - but perhaps we should simply do this when we fix the remaining types, seeing as this will also require people to use a somewhat recent TS version (symbols were introduced in 2.7 as far as I can see).

from relay-compiler-language-typescript.

So symbols don’t show up? And is that documented behaviour?

I do like replacing the enums with symbols, I was also just thinking about how we’re using Babel 7 for some of our compilation needs in a web app that we’ll be using Relay Modern in soon and the Babel 7 TS parser does not allow const enum.

from relay-compiler-language-typescript.

So symbols don’t show up? And is that documented behaviour?

They don't show up, but it is, as far as I can see, not documented - but think of the consequences it would have if they showed up - type []. and suddenly a wealth of symbols would show up. We could ask the TS team about this - but I'd say it's fairly safe to say they will not show up.

I do like replacing the enums with symbols, I was also just thinking about how we’re using Babel 7 for some of our compilation needs in a web app that we’ll be using Relay Modern in soon and the Babel 7 TS parser does not allow const enum.

That is a bummer. Don't you compile TS using the TS compiler before passing it to babel though? (More or less, set target language in tsconfig to ES2015 to leave the tagged template literals in the compiled code).

On an unrelated note, i sent you an email yesterday in the email listed on your profile (not sure if you actively use that).

from relay-compiler-language-typescript.

I'd say it's fairly safe to say they will not show up.

Fair enough, I think you’re right 👍

Don't you compile TS using the TS compiler before passing it to babel though?

We do right now, but in the cases where we really can’t get away from using Babel we’d prefer to optimise the performance by having 1 compiler instead of multiple.

On an unrelated note, i sent you an email yesterday in the email listed on your profile (not sure if you actively use that).

Hah, I had just replied to that previously :)

from relay-compiler-language-typescript.

We do right now, but in the cases where we really can’t get away from using Babel we’d prefer to optimise the performance by having 1 compiler instead of multiple.

Makes sense. I have done some initial testing on our own codebase, for performance considerations, what getting rid of babel for TS code would mean. We found a rough 25% of the build time was removed by getting rid of babel (babel 6.5.x). It might be worth it testing the transformation plugin up in the PR for you guys. For Relay modern I can't imagine it having any real issues.

EDIT: forgot to mention, we have a pipeline of both TS and babel currently. I have not tested what dropping TS means, though my gut feeling tells me that the TS loader is much much faster than the babel loader. This comes from ie. running our example code with the TS transformation stuff I made - which ran way faster.

from relay-compiler-language-typescript.

Alas we can’t get rid of Babel that easily in our React Native codebase, as React Native itself comes as Flow annotated source (not transpiled) and its packager has many assumptions. I would definitely believe TSC to be faster, even just from the fact that people at MS are paid to work on it.

from relay-compiler-language-typescript.

Yeah TS and React Native are yet to become great friends. We have done very little work on React Native (and I haven't been that involved with it), but I do believe we're compiling from TS to JS before letting babel do its thing.

from relay-compiler-language-typescript.

You mean statically? We have our own custom RN transformer that uses TSC to transpile on the fly before passing on to Babel https://github.com/artsy/emission/blob/3a415954b7b163703a5ed9fda41e7053de681469/transformer/index.js

from relay-compiler-language-typescript.

Sweet, I'll pass that along to our RN guy :) thx!

from relay-compiler-language-typescript.

Would recommend https://github.com/ds300/react-native-typescript-transformer which was based on our code (also has the babel feature) and generally supports more than one version of RN. I have a PR moving Emission (our RN) to it

from relay-compiler-language-typescript.

Yup, good point @orta 👍

from relay-compiler-language-typescript.