-
-
Notifications
You must be signed in to change notification settings - Fork 206
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
(feat) ComponentEvents interface #459
Changes from 1 commit
f1ae8f9
d1337be
b4fb823
aa59021
36bbfbe
f600092
fe263a5
c33dbcf
54fa9d7
819dac2
a2b148d
0159f8a
464fbf4
47af2e8
c79d3c7
9994e76
582fef4
5f56ad9
8a2cab0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,6 @@ | ||
import ts from 'typescript'; | ||
import { EventHandler } from './event-handler'; | ||
import { getVariableAtTopLevel } from '../utils/tsAst'; | ||
|
||
export abstract class ComponentEvents { | ||
protected events = new Map<string, { type: string; doc?: string }>(); | ||
|
@@ -32,7 +33,7 @@ export class ComponentEventsFromInterface extends ComponentEvents { | |
const map = new Map<string, { type: string; doc?: string }>(); | ||
|
||
node.members.filter(ts.isPropertySignature).forEach((member) => { | ||
map.set(member.name.getText(), { | ||
map.set(this.getName(member.name), { | ||
type: member.type?.getText() || 'Event', | ||
doc: this.getDoc(node, member), | ||
}); | ||
|
@@ -41,6 +42,52 @@ export class ComponentEventsFromInterface extends ComponentEvents { | |
return map; | ||
} | ||
|
||
private getName(prop: ts.PropertyName) { | ||
if (ts.isIdentifier(prop) || ts.isStringLiteral(prop)) { | ||
jasonlyu123 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
return prop.text; | ||
} | ||
|
||
if (ts.isComputedPropertyName(prop)) { | ||
if (ts.isIdentifier(prop.expression)) { | ||
const identifierName = prop.expression.text; | ||
const identifierValue = this.getIdentifierValue(prop, identifierName); | ||
if (!identifierValue) { | ||
this.throwError(prop); | ||
} | ||
return identifierValue; | ||
} | ||
} | ||
|
||
this.throwError(prop); | ||
} | ||
|
||
private getIdentifierValue(prop: ts.ComputedPropertyName, identifierName: string) { | ||
const variable = getVariableAtTopLevel(prop.getSourceFile(), identifierName); | ||
if (variable && ts.isStringLiteral(variable.initializer)) { | ||
return variable.initializer.text; | ||
} | ||
} | ||
|
||
private throwError(prop: ts.PropertyName) { | ||
const error: any = new Error( | ||
'The ComponentEvents interface can only have properties of type ' + | ||
'Identifier, StringLiteral or ComputedPropertyName. ' + | ||
'In case of ComputedPropertyName, ' + | ||
'it must be a const declared within the component and initialized with a string.', | ||
); | ||
error.start = toLineColumn(prop.getStart()); | ||
error.end = toLineColumn(prop.getEnd()); | ||
throw error; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This throws if the interface definition does not match our expextations. I hope this makes it clear to the user what is possible and what is not. |
||
|
||
function toLineColumn(pos: number) { | ||
const lineChar = prop.getSourceFile().getLineAndCharacterOfPosition(pos); | ||
return { | ||
line: lineChar.line + 1, | ||
column: lineChar.character, | ||
}; | ||
} | ||
} | ||
|
||
private getDoc(node: ts.InterfaceDeclaration, member: ts.PropertySignature) { | ||
let doc = undefined; | ||
const comment = ts.getLeadingCommentRanges( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,15 +16,20 @@ describe('svelte2tsx', () => { | |
} | ||
|
||
(solo ? it.only : it)(dir, () => { | ||
const input = fs.readFileSync(`${__dirname}/samples/${dir}/input.svelte`, 'utf-8').replace(/\s+$/, '').replace(/\r\n/g, "\n"); | ||
const expectedOutput = fs.readFileSync(`${__dirname}/samples/${dir}/expected.tsx`, 'utf-8').replace(/\s+$/, '').replace(/\r\n/g, "\n"); | ||
const path = `${__dirname}/samples/${dir}`; | ||
const input = fs.readFileSync(`${path}/input.svelte`, 'utf-8').replace(/\s+$/, '').replace(/\r\n/g, "\n"); | ||
const expectedOutput = fs.readFileSync(`${path}/expected.tsx`, 'utf-8').replace(/\s+$/, '').replace(/\r\n/g, "\n"); | ||
const expecterOtherOutput = fs.existsSync(`${path}/expected.js`) && require(`${path}/expected`); | ||
|
||
const { map, code} = svelte2tsx(input, { | ||
const output = svelte2tsx(input, { | ||
strictMode: dir.includes('strictMode'), | ||
isTsFile: dir.startsWith('ts-'), | ||
filename: 'input.svelte' | ||
}); | ||
assert.equal(code, expectedOutput); | ||
assert.equal(output.code, expectedOutput); | ||
if (expecterOtherOutput) { | ||
expecterOtherOutput(output); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I enhanced the test suite so you can test other outputs of |
||
} | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
let assert = require('assert') | ||
|
||
module.exports = function ({events}) { | ||
assert.deepEqual( | ||
events.getAll(), | ||
[ | ||
{name: 'a', type: 'boolean', doc: ' Some doc'}, | ||
{name: 'b', type: 'string', doc: undefined}, | ||
{name: 'c', type: 'Event', doc: undefined} | ||
] | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
///<reference types="svelte" /> | ||
<></>;function render() { | ||
|
||
const A = 'a'; | ||
const B = 'b', C = 'c'; | ||
interface ComponentEvents { | ||
/** | ||
* Some doc | ||
*/ | ||
[A]: boolean; | ||
[B]: string; | ||
[C]; | ||
} | ||
; | ||
() => (<></>); | ||
return { props: {}, slots: {}, getters: {}, events: {} as unknown as ComponentEvents }} | ||
|
||
export default class Input__SvelteComponent_ extends createSvelte2TsxComponent(__sveltets_partial(render)) { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
<script lang="ts"> | ||
const A = 'a'; | ||
const B = 'b', C = 'c'; | ||
interface ComponentEvents { | ||
/** | ||
* Some doc | ||
*/ | ||
[A]: boolean; | ||
[B]: string; | ||
[C]; | ||
} | ||
</script> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
let assert = require('assert') | ||
|
||
module.exports = function ({events}) { | ||
assert.deepEqual( | ||
events.getAll(), | ||
[ | ||
{name: 'a-b', type: 'boolean', doc: ' Some doc'}, | ||
{name: 'b', type: 'string', doc: undefined}, | ||
{name: 'c', type: 'Event', doc: undefined} | ||
] | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
///<reference types="svelte" /> | ||
<></>;function render() { | ||
|
||
interface ComponentEvents { | ||
/** | ||
* Some doc | ||
*/ | ||
'a-b': boolean; | ||
'b': string; | ||
'c'; | ||
} | ||
; | ||
() => (<></>); | ||
return { props: {}, slots: {}, getters: {}, events: {} as unknown as ComponentEvents }} | ||
|
||
export default class Input__SvelteComponent_ extends createSvelte2TsxComponent(__sveltets_partial(render)) { | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
<script lang="ts"> | ||
interface ComponentEvents { | ||
/** | ||
* Some doc | ||
*/ | ||
'a-b': boolean; | ||
'b': string; | ||
'c'; | ||
} | ||
</script> |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
let assert = require('assert') | ||
|
||
module.exports = function ({events}) { | ||
assert.deepEqual( | ||
events.getAll(), | ||
[ | ||
{name: 'a', type: 'boolean', doc: ' Some doc'}, | ||
{name: 'b', type: 'string', doc: undefined}, | ||
{name: 'c', type: 'Event', doc: undefined} | ||
] | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,9 +5,9 @@ | |
/** | ||
* Some doc | ||
*/ | ||
a: boolean; | ||
b: string; | ||
c; | ||
a: boolean; | ||
b: string; | ||
c; | ||
} | ||
; | ||
() => (<></>); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,8 +3,8 @@ | |
/** | ||
* Some doc | ||
*/ | ||
a: boolean; | ||
b: string; | ||
c; | ||
a: boolean; | ||
b: string; | ||
c; | ||
} | ||
</script> |
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 name here is possibly a string literal or a computed property. i.e.
getText
in either situation would be wrong. if it's a string literal the text can be extracted frommember.name.text
.About the computed property, from my testing, computed event handler like <Component on:[SOME_CONSTANT] /> doesn't work. Maybe we could make it works here but filter out in the completion. Or simply doesn't support it as it doesn't make much sense when you can't use it in markup.
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.
Good catch! We should definitely support string literals. About the constant, we could go to the root and check if it's defined there and get the value from there - so we at least support it a little. I will also add docs about this.