Skip to content

Commit 5b947e6

Browse files
authored
feat(47619): add support for extracting jsx string literal attribute to a constant (microsoft#47624)
1 parent 66dba13 commit 5b947e6

File tree

4 files changed

+103
-4
lines changed

4 files changed

+103
-4
lines changed

src/services/refactors/extractSymbol.ts

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -433,7 +433,7 @@ namespace ts.refactor.extractSymbol {
433433
// For understanding how skipTrivia functioned:
434434
Debug.assert(!positionIsSynthesized(nodeToCheck.pos), "This failure could trigger https://github.com/Microsoft/TypeScript/issues/20809 (2)");
435435

436-
if (!isStatement(nodeToCheck) && !(isExpressionNode(nodeToCheck) && isExtractableExpression(nodeToCheck))) {
436+
if (!isStatement(nodeToCheck) && !(isExpressionNode(nodeToCheck) && isExtractableExpression(nodeToCheck)) && !isStringLiteralJsxAttribute(nodeToCheck)) {
437437
return [createDiagnosticForNode(nodeToCheck, Messages.statementOrExpressionExpected)];
438438
}
439439

@@ -625,13 +625,16 @@ namespace ts.refactor.extractSymbol {
625625
if (isStatement(node)) {
626626
return [node];
627627
}
628-
else if (isExpressionNode(node)) {
628+
if (isExpressionNode(node)) {
629629
// If our selection is the expression in an ExpressionStatement, expand
630630
// the selection to include the enclosing Statement (this stops us
631631
// from trying to care about the return value of the extracted function
632632
// and eliminates double semicolon insertion in certain scenarios)
633633
return isExpressionStatement(node.parent) ? [node.parent] : node as Expression;
634634
}
635+
if (isStringLiteralJsxAttribute(node)) {
636+
return node;
637+
}
635638
return undefined;
636639
}
637640

@@ -1649,7 +1652,7 @@ namespace ts.refactor.extractSymbol {
16491652
// Unfortunately, this code takes advantage of the knowledge that the generated method
16501653
// will use the contextual type of an expression as the return type of the extracted
16511654
// method (and will therefore "use" all the types involved).
1652-
if (inGenericContext && !isReadonlyArray(targetRange.range)) {
1655+
if (inGenericContext && !isReadonlyArray(targetRange.range) && !isJsxAttribute(targetRange.range)) {
16531656
const contextualType = checker.getContextualType(targetRange.range)!; // TODO: GH#18217
16541657
recordTypeParameterUsages(contextualType);
16551658
}
@@ -1997,6 +2000,11 @@ namespace ts.refactor.extractSymbol {
19972000
}
19982001

19992002
function isInJSXContent(node: Node) {
2000-
return (isJsxElement(node) || isJsxSelfClosingElement(node) || isJsxFragment(node)) && (isJsxElement(node.parent) || isJsxFragment(node.parent));
2003+
return isStringLiteralJsxAttribute(node) ||
2004+
(isJsxElement(node) || isJsxSelfClosingElement(node) || isJsxFragment(node)) && (isJsxElement(node.parent) || isJsxFragment(node.parent));
2005+
}
2006+
2007+
function isStringLiteralJsxAttribute(node: Node): node is StringLiteral {
2008+
return isStringLiteral(node) && node.parent && isJsxAttribute(node.parent);
20012009
}
20022010
}
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @jsx: preserve
4+
// @filename: a.tsx
5+
////function Foo() {
6+
//// return (
7+
//// <div>
8+
//// <a href=/*a*/"string"/*b*/></a>;
9+
//// </div>
10+
//// );
11+
////}
12+
13+
goTo.file("a.tsx");
14+
goTo.select("a", "b");
15+
edit.applyRefactor({
16+
refactorName: "Extract Symbol",
17+
actionName: "constant_scope_1",
18+
actionDescription: "Extract to constant in global scope",
19+
newContent:
20+
`const /*RENAME*/newLocal = "string";
21+
function Foo() {
22+
return (
23+
<div>
24+
<a href={newLocal}></a>;
25+
</div>
26+
);
27+
}`
28+
});
Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @jsx: preserve
4+
// @filename: a.tsx
5+
////function Foo() {
6+
//// return (
7+
//// <div>
8+
//// <a href=/*a*/"string"/*b*/></a>;
9+
//// </div>
10+
//// );
11+
////}
12+
13+
goTo.file("a.tsx");
14+
goTo.select("a", "b");
15+
edit.applyRefactor({
16+
refactorName: "Extract Symbol",
17+
actionName: "constant_scope_0",
18+
actionDescription: "Extract to constant in enclosing scope",
19+
newContent:
20+
`function Foo() {
21+
const /*RENAME*/newLocal = "string";
22+
return (
23+
<div>
24+
<a href={newLocal}></a>;
25+
</div>
26+
);
27+
}`
28+
});
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
/// <reference path='fourslash.ts' />
2+
3+
// @jsx: preserve
4+
// @filename: a.tsx
5+
////declare var React: any;
6+
////class Foo extends React.Component<{}, {}> {
7+
//// render() {
8+
//// return (
9+
//// <div>
10+
//// <a href=/*a*/"string"/*b*/></a>;
11+
//// </div>
12+
//// );
13+
//// }
14+
////}
15+
16+
goTo.file("a.tsx");
17+
goTo.select("a", "b");
18+
edit.applyRefactor({
19+
refactorName: "Extract Symbol",
20+
actionName: "constant_scope_1",
21+
actionDescription: "Extract to readonly field in class 'Foo'",
22+
newContent:
23+
`declare var React: any;
24+
class Foo extends React.Component<{}, {}> {
25+
private readonly newProperty = "string";
26+
27+
render() {
28+
return (
29+
<div>
30+
<a href={this./*RENAME*/newProperty}></a>;
31+
</div>
32+
);
33+
}
34+
}`
35+
});

0 commit comments

Comments
 (0)