GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Add support for JSX fragment syntax by uniqueiniquity · Pull Request #19249 · microsoft/TypeScript

Conversation

@uniqueiniquity

@uniqueiniquity Ben Lichtman (uniqueiniquity) commented Oct 17, 2017

Copy link
Copy Markdown
Contributor

Fixes #19094, which in turn reflects react/jsx#93.

This PR adds support for the <>...</> syntax for fragments in JSX. Under preserve emit, the syntax is maintained, and under react emit, fragment blocks are transformed into React.createElement(React.Fragment, null, ...) (and are thus affected by --jsxFactory). Due to similarities with the JSXElement syntax, the implementation shares much of the same code where appropriate.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
styfle, thomasd16, tkrotoff, mihailik, tills13, zwenza, and rosalyntg reacted with thumbs up emoji mihailik, tills13, pocotan001, and rosalyntg reacted with heart emoji
All reactions
  • 👍 7 reactions
  • ❤️ 4 reactions
Ben Lichtman (uniqueiniquity) added 4 commits October 17, 2017 09:38
@uniqueiniquity
Add support for JSX fragment syntax
abb3f58
@uniqueiniquity
Update tests
269d37a
@uniqueiniquity
Add baselines
5f8a1b5
@uniqueiniquity
Update API baselines
845f573
@uniqueiniquity
Ben Lichtman (uniqueiniquity) requested review from Ryan Cavanaugh (RyanCavanaugh) and Bill Ticehurst (billti) October 17, 2017 16:52
RyanCavanaugh
Ryan Cavanaugh (RyanCavanaugh) reviewed Oct 17, 2017
View reviewed changes
Comment thread src/compiler/checker.ts Outdated
// which is a type of the parameter of the signature we are trying out.
// If there is no contextual type (e.g. we are trying to resolve stateful component), get attributes type from resolving element's tagName
const attributesType = getContextualType(jsxAttributes);
if (jsxAttributes) {

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Oct 17, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just do an early return of undefined here if jsxAttributes is undefined

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@uniqueiniquity Ben Lichtman (uniqueiniquity) Oct 18, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
RyanCavanaugh
Ryan Cavanaugh (RyanCavanaugh) reviewed Oct 17, 2017
View reviewed changes
Comment thread src/compiler/checker.ts

function checkJsxOpeningLikeElement(node: JsxOpeningLikeElement) {
checkGrammarJsxElement(node);
function checkJsxOpeningLikeElementOrOpeningFragment(node: JsxOpeningLikeElement | JsxOpeningFragment) {

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Oct 17, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No need to rename this - a fragment is an element (IMHO?)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
RyanCavanaugh reacted with thumbs down emoji
All reactions
  • 👎 1 reaction

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Oct 17, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I take it back

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
RyanCavanaugh
Ryan Cavanaugh (RyanCavanaugh) reviewed Oct 17, 2017
View reviewed changes
Comment thread src/compiler/parser.ts Outdated
else if (opening.kind === SyntaxKind.JsxOpeningFragment) {
const node = <JsxFragment>createNode(SyntaxKind.JsxFragment, opening.pos);
node.openingFragment = opening;

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Oct 17, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit extra newline

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@uniqueiniquity Ben Lichtman (uniqueiniquity) Oct 18, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
RyanCavanaugh
Ryan Cavanaugh (RyanCavanaugh) reviewed Oct 17, 2017
View reviewed changes
Comment thread src/compiler/parser.ts Outdated
// If we hit EOF, issue the error at the tag that lacks the closing element
// rather than at the end of the file (which is useless)
parseErrorAtPosition(openingTagName.pos, openingTagName.end - openingTagName.pos, Diagnostics.JSX_element_0_has_no_corresponding_closing_tag, getTextOfNodeFromSourceText(sourceText, openingTagName));
if (isJsxOpeningElement(openingTag)) {

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Oct 17, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading this whole thing it seems like it'd be clearer if we wrote e.g. if (isJsxFragment(openingTag)) instead of isJsxOpeningElement since the former is more the thing we're special-casing

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@uniqueiniquity Ben Lichtman (uniqueiniquity) Oct 18, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Restructured.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
RyanCavanaugh
Ryan Cavanaugh (RyanCavanaugh) reviewed Oct 17, 2017
View reviewed changes
Comment thread src/compiler/parser.ts
return finishNode(node);
}

function parseJsxClosingFragment(inExpressionContext: boolean): JsxClosingFragment {

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Oct 17, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reading the spec, I don't see how we'd legally get into the inExpressionContext = false case since you can't write <foo><><div /></></foo>

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@uniqueiniquity Ben Lichtman (uniqueiniquity) Oct 17, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec changed, now you can.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@uniqueiniquity Ben Lichtman (uniqueiniquity) Oct 17, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

react/jsx#97

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
RyanCavanaugh
Ryan Cavanaugh (RyanCavanaugh) reviewed Oct 17, 2017
View reviewed changes
Comment thread src/compiler/scanner.ts Outdated

/* @internal */
export function tokenIsIdentifierOrKeywordOrGreaterThan(token: SyntaxKind): boolean {
return token === SyntaxKind.GreaterThanToken || token >= SyntaxKind.Identifier;

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Oct 17, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Call tokenIsIdentifierOrKeyword in the right arm in case that logic changes

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@uniqueiniquity Ben Lichtman (uniqueiniquity) Oct 18, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
RyanCavanaugh
Ryan Cavanaugh (RyanCavanaugh) reviewed Oct 17, 2017
View reviewed changes
Comment thread src/compiler/types.ts Outdated
}

/// Either the opening tag in a <Tag>...</Tag> pair, or the lone <Tag /> in a self-closing form
/// Either the opening tag in a <Tag>...</Tag> pair, the opening tag in a <>...</> pair, or the lone <Tag /> in a self-closing form

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Oct 17, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment doesn't match our usage/terminology of isJsxOpeningLikeElement

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@uniqueiniquity Ben Lichtman (uniqueiniquity) Oct 18, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
RyanCavanaugh
Ryan Cavanaugh (RyanCavanaugh) reviewed Oct 17, 2017
View reviewed changes
Comment thread tests/baselines/reference/checkJsxChildrenProperty14.errors.txt
}

// OK
let k1 = <Comp a={10} b="hi"><></><Button /><AnotherButton /></Comp>;

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Oct 17, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be a parse error

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Oct 17, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or perhaps we should parse it leniently but issue a grammar error in checking

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Oct 17, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(disregard)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
Jessidhia reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@uniqueiniquity
Respond to CR
7566529
@uniqueiniquity

Ben Lichtman (uniqueiniquity) commented Oct 17, 2017

Copy link
Copy Markdown
Contributor Author

Mohamed Hegazy (@mhegazy) could you take a look at this?
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@uniqueiniquity

Ben Lichtman (uniqueiniquity) commented Oct 18, 2017

Copy link
Copy Markdown
Contributor Author

Editor actions work as expected in VS and VS Code (i.e., go to def, find all references, etc are basically no-ops or return no results). I will be updating the syntax highlighter accordingly.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
RyanCavanaugh
Ryan Cavanaugh (RyanCavanaugh) reviewed Oct 18, 2017
View reviewed changes
Comment thread src/compiler/checker.ts Outdated
// If there is no contextual type (e.g. we are trying to resolve stateful component), get attributes type from resolving element's tagName
const attributesType = getContextualType(jsxAttributes);
if (!jsxAttributes) {
return anyType; // don't check children of a fragment

@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Oct 18, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's better to return undefined than anyType here because it will cause the calling code to bail out earlier

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@uniqueiniquity Ben Lichtman (uniqueiniquity) Oct 18, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@uniqueiniquity

Ben Lichtman (uniqueiniquity) commented Oct 18, 2017

Copy link
Copy Markdown
Contributor Author

Tracking syntax highlighter changes for VS Code at microsoft/TypeScript-TmLanguage#534.
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
RyanCavanaugh
Ryan Cavanaugh (RyanCavanaugh) approved these changes Oct 18, 2017
View reviewed changes
@uniqueiniquity
Respond to CR
a7f9ec0
Jessidhia
Jessica Franco (Jessidhia) reviewed Oct 19, 2017
View reviewed changes
Comment thread src/compiler/factory.ts
const tagName = createPropertyAccess(
createReactNamespace(reactNamespace, parentElement),
"Fragment"
);

@Jessidhia Jessica Franco (Jessidhia) Oct 19, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this perhaps be customizable, similar to how --jsxFactory is? e.g. --jsxFragmentComponent?

Arguably not useful for anything but React and React clones, though; as other less similar frameworks would want to do other special processing on fragments.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@uniqueiniquity Ben Lichtman (uniqueiniquity) Oct 19, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with your second point here; to take the example of Mithril, the framework supports fragments, but with a completely different approach than React will (i.e. m.fragment(attrs, children)) so it's not as convenient as simply replacing the call to createElement with --jsxFactory. It seems to make sense in this case to just preserve the JSX syntax as is for non-React libs and have each define their own transpilation depending on their implementation of fragments.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Oct 23, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kovensky do you know what babel will do here? would it support another version of the paragma for the React.createElement?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Oct 24, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

checked with Henry (@hzoo) over slack and does not seem that babel have plans for this in the immediate term.
I wounder if we should just make it an error to use Fragment + --jsx React + --jsxNamespace

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@mhegazy Mohamed Hegazy (mhegazy) Oct 24, 2017

Copy link
Copy Markdown
Contributor

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ben Lichtman (@uniqueiniquity) can you file an issue for adding the error.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Oct 24, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, I think I see now. I'm confusing jsxFactory and reactNamespace

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
uniqueiniquity reacted with thumbs up emoji
All reactions
  • 👍 1 reaction

@uniqueiniquity Ben Lichtman (uniqueiniquity) Oct 24, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So are you on board with issuing the error?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Oct 25, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably, but feel free not to block this PR on it.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@uniqueiniquity Ben Lichtman (uniqueiniquity) Oct 25, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated the PR to issue an error.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@clemmy Clement Hoang (clemmy) Oct 31, 2017

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the Babel PR, I add a pragma for fragments, as Mohamed Hegazy (@mhegazy) suggested. https://github.com/babel/babel/pull/6552/files#diff-7e5b807d66ce513f501563ad8f8d03e6R117
https://github.com/babel/babel/pull/6552/files#diff-7e5b807d66ce513f501563ad8f8d03e6R89

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Jessidhia
Jessica Franco (Jessidhia) reviewed Oct 19, 2017
View reviewed changes
Comment thread tests/baselines/reference/checkJsxChildrenProperty14.js
}

// OK
let k4 = <SingleChildComp a={10} b="hi"><><Button /><AnotherButton /></></SingleChildComp>;

@Jessidhia Jessica Franco (Jessidhia) Oct 19, 2017

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd kinda prefer for these to be a different type like, say, JSX.Fragment, but having one would not be useful anyway without nominal typing.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@weswigham Wesley Wigham (weswigham) Oct 25, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We'd like to make JSX.Element generic in the future - possibly looking it up and instantiating it based on the return type of the factory function; that would handle the Fragment case somewhat elegantly.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@SMotaal Saleh Abdel Motaal (SMotaal) Nov 14, 2017 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generic JSX.Element<T> would be great. Outside of the React bubble, TypeScript-oriented developers like myself often wonder why the disconnect.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
mhegazy
Mohamed Hegazy (mhegazy) approved these changes Oct 24, 2017
View reviewed changes
DanielRosenwasser
Daniel Rosenwasser (DanielRosenwasser) requested changes Oct 24, 2017
View reviewed changes
Comment thread src/compiler/diagnosticMessages.json Outdated
"category": "Error",
"code": 17014
},
"Expected corresponding JSX fragment closing tag.": {

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Oct 24, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Expected corresponding closing tag for JSX fragment.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@uniqueiniquity Ben Lichtman (uniqueiniquity) Oct 24, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Updated.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
Comment thread src/compiler/emitter.ts Outdated

if (isJsxOpeningElement(node)) {
emitJsxTagName(node.tagName);
writeIfAny(node.attributes.properties, " ");

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Oct 24, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should move this into the below if check and replace the writeIfAny with a write

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@uniqueiniquity Ben Lichtman (uniqueiniquity) Oct 24, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Moved.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
@DanielRosenwasser

Daniel Rosenwasser (DanielRosenwasser) commented Oct 24, 2017

Copy link
Copy Markdown
Member

Looks reasonable apart from the nits I requested changes on. 👍
All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
@uniqueiniquity
Respond to CR
fb05886
DanielRosenwasser
Daniel Rosenwasser (DanielRosenwasser) reviewed Oct 24, 2017
View reviewed changes
Comment thread src/compiler/checker.ts Outdated
function getContextualTypeForJsxExpression(node: JsxExpression): Type {
// JSX expression can appear in two position : JSX Element's children or JSX attribute
const jsxAttributes = isJsxAttributeLike(node.parent) ?
const jsxAttributes: JsxAttributes = isJsxAttributeLike(node.parent) ?

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) Oct 24, 2017

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do you need this type annotation?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions

@uniqueiniquity Ben Lichtman (uniqueiniquity) Oct 25, 2017

Copy link
Copy Markdown
Contributor Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.
All reactions
DanielRosenwasser
Daniel Rosenwasser (DanielRosenwasser) approved these changes Oct 24, 2017
View reviewed changes
@uniqueiniquity
Remove unnecessary type annotation
961495b
@duailibe Lucas Duailibe (duailibe) mentioned this pull request Nov 27, 2017
Update TypeScript to 2.6.2 prettier/prettier#3331
Merged
This was referenced Nov 28, 2017
Update typescript in / from 2.6.1 to 2.6.2 mjohnson9/twitch-markov-bot#57
Open
Update typescript in /bundle from 2.4.2 to 2.6.2 src-zone/material#98
Closed
Update 35 dependencies from npm storybookjs/storybook#2429
Merged
@pelotom Tom Crockett (pelotom) mentioned this pull request Dec 6, 2017
jsx-wrap-multiline can't handle JSX fragment syntax palantir/tslint#3547
Closed
@ghost ghost mentioned this pull request Dec 19, 2017
Update typescript in / from 2.5.1 to 2.6.2 chauncey-garrett/dotfiles#63
Open
@ghost ghost mentioned this pull request Jan 22, 2018
Debug Failure. False expression: JsxText tokens should not be the first child of JsxElement | JsxSelfClosingElement in findRightmostChildNodeWithTokens during getQuickInfoAtPosition #20788
Closed
This was referenced Feb 2, 2018
chore(deps): update dependency typescript to v2.7.2 - autoclosed motorcycle/starter#38
Closed
Update dependency typescript to v2.9.2 blerp/isomorphic-piwik#25
Open
chore(deps): update dependency typescript to v2.9.2 TylorS/typed-history#15
Open
chore(deps): update dependency typescript to v2.7.2 TylorS/typed-unmaintained#51
Closed
chore(deps): update dependency typescript to v2.7.2 - autoclosed motorcycle/react-dom#43
Closed
chore(deps): update dependency typescript to v2.9.2 motorcycle/compiler#51
Open
chore(deps): update dependency typescript to v2.9.2 ikatyang/generator-ts-jest#101
Closed
chore(deps): update dependency typescript to v2.7.1 apollographql/apollo-client#2940
Closed
chore(deps): update dependency typescript to v2.8.3 ikatyang/dts-element-fp#76
Merged
chore(deps): update dependency typescript to v2.7.2 - autoclosed Urigo/meteor-rxjs#135
Closed
chore(deps): update dependency typescript to v2.8.1 TylorS/typed-stream#25
Closed
Update dependency typescript to v2.7.2 typestyled/core#16
Merged
chore(deps): update dependency typescript to v2.9.2 motorcycle/loader#12
Open
chore(deps): update dependency typescript to v2.9.2 motorcycle/motorcycle.ts#249
Open
Update dependency typescript to v2.9.2 Urigo/leumi-leumicard-bank-data-scraper#6
Open
build: update dependency typescript to ~2.7.1 sparkles-dev/ng-content-driven-angular#46
Merged
chore: update typescript to version ~2.8.1 dherges/dgeni-ngx#12
Open
fix(deps): update dependency typescript to ~2.8.1 ikatyang/dts-element#45
Closed
@renovate-tests Tests (renovate-tests) mentioned this pull request Feb 5, 2018
Update dependency typescript to v2.7.1 renovate-tests/slup#16
Open
@manucorporat Manu MA (manucorporat) mentioned this pull request Feb 14, 2018
feat(render): support JSX fragment stenciljs/core#534
Closed
@devuxer Dan M. (devuxer) mentioned this pull request Mar 2, 2018
React.Fragment works but the short syntax (<>...</>) doesn't s-panferov/awesome-typescript-loader#542
Closed
@dependabot-preview dependabot-preview Bot mentioned this pull request Mar 13, 2018
Bump typescript from 2.3.4 to 2.7.2 in /frontend Xenira/Shashki#64
Merged
@microsoft Microsoft (microsoft) locked and limited conversation to collaborators Jun 14, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@DanielRosenwasser Daniel Rosenwasser (DanielRosenwasser) DanielRosenwasser approved these changes
@weswigham Wesley Wigham (weswigham) weswigham left review comments
@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) RyanCavanaugh approved these changes
@billti Bill Ticehurst (billti) Awaiting requested review from billti
+5 more reviewers
@Jessidhia Jessica Franco (Jessidhia) Jessidhia left review comments
@hzoo Henry (hzoo) hzoo left review comments
@SMotaal Saleh Abdel Motaal (SMotaal) SMotaal left review comments
@clemmy Clement Hoang (clemmy) clemmy left review comments
@mhegazy Mohamed Hegazy (mhegazy) mhegazy approved these changes
Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Uh oh!

There was an error while loading. Please reload this page.

10 participants

@uniqueiniquity @DanielRosenwasser @styfle @mhegazy @Jessidhia @hzoo @SMotaal @weswigham @clemmy @RyanCavanaugh
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.