Apply suggestions from code review

Co-authored-by: Rich Harris <rich.harris@vercel.com>
pull/10851/head
Simon H 2 years ago committed by GitHub
parent 7992ef84be
commit ef0bc44023
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194

@ -182,7 +182,7 @@ const runes = {
`$props() assignment must not contain nested properties or computed keys`, `$props() assignment must not contain nested properties or computed keys`,
'invalid-props-location': () => 'invalid-props-location': () =>
`$props() can only be used at the top level of components as a variable declaration initializer`, `$props() can only be used at the top level of components as a variable declaration initializer`,
'invalid-bindable-location': () => `$bindable() can only be used as part of the $props() rune`, 'invalid-bindable-location': () => `$bindable() can only be used inside a $props() declaration`,
/** @param {string} rune */ /** @param {string} rune */
'invalid-state-location': (rune) => 'invalid-state-location': (rune) =>
`${rune}(...) can only be used as a variable declaration initializer or a class field`, `${rune}(...) can only be used as a variable declaration initializer or a class field`,

@ -1073,7 +1073,7 @@ export const validation_runes = merge(validation, a11y_validators, {
} else if (rune === '$state' && args.length > 1) { } else if (rune === '$state' && args.length > 1) {
error(node, 'invalid-rune-args-length', rune, [0, 1]); error(node, 'invalid-rune-args-length', rune, [0, 1]);
} else if (rune === '$props') { } else if (rune === '$props') {
if (rune === '$props' && state.has_props_rune) { if (state.has_props_rune) {
error(node, 'duplicate-props-rune'); error(node, 'duplicate-props-rune');
} }

@ -270,15 +270,6 @@ export function client_component(source, analysis, options) {
if (analysis.accessors) { if (analysis.accessors) {
for (const [name, binding] of properties) { for (const [name, binding] of properties) {
const key = binding.prop_alias ?? name; const key = binding.prop_alias ?? name;
if (
binding.kind === 'prop' &&
[...analysis.instance.scope.declarations].some(
([name, d]) => d.kind === 'bindable_prop' && (d.prop_alias ?? name) === key
)
) {
// bindable prop takes precedence
continue;
}
component_returned_object.push( component_returned_object.push(
b.get(key, [b.return(b.call(b.id(name)))]), b.get(key, [b.return(b.call(b.id(name)))]),

@ -211,19 +211,25 @@ export const javascript_visitors_runes = {
property.value.type === 'AssignmentPattern' ? property.value.left : property.value; property.value.type === 'AssignmentPattern' ? property.value.left : property.value;
assert.equal(id.type, 'Identifier'); assert.equal(id.type, 'Identifier');
const binding = /** @type {import('#compiler').Binding} */ (state.scope.get(id.name)); const binding = /** @type {import('#compiler').Binding} */ (state.scope.get(id.name));
let initial = /** @type {import('estree').Expression | null} */ (binding.initial); const initial =
if (initial) { binding.initial &&
initial = /** @type {import('estree').Expression} */ (visit(initial)); /** @type {import('estree').Expression} */ (visit(binding.initial));
}
if (binding.reassigned || state.analysis.accessors || initial) { if (binding.reassigned || state.analysis.accessors || initial) {
declarations.push(b.declarator(id, get_prop_source(binding, state, name, initial))); declarations.push(b.declarator(id, get_prop_source(binding, state, name, initial)));
} }
} else { } else {
// RestElement // RestElement
/** @type {import('estree').Expression[]} */ declarations.push(
const args = [b.id('$$props'), b.array(seen.map((name) => b.literal(name)))]; b.declarator(
declarations.push(b.declarator(property.argument, b.call('$.rest_props', ...args))); property.argument,
b.call(
'$.rest_props',
b.id('$$props'),
b.array(seen.map((name) => b.literal(name)))
)
)
);
} }
} }

@ -241,7 +241,7 @@ export interface Binding {
node: Identifier; node: Identifier;
/** /**
* - `normal`: A variable that is not in any way special * - `normal`: A variable that is not in any way special
* - `prop`: A normal prop (possibly reassigned) * - `prop`: A normal prop (possibly reassigned or mutated)
* - `bindable_prop`: A prop one can `bind:` to (possibly reassigned or mutated) * - `bindable_prop`: A prop one can `bind:` to (possibly reassigned or mutated)
* - `rest_prop`: A rest prop * - `rest_prop`: A rest prop
* - `state`: A state variable * - `state`: A state variable

@ -1279,7 +1279,8 @@ if (DEV) {
throw_rune_error('$effect', ['pre', 'root', 'active']); throw_rune_error('$effect', ['pre', 'root', 'active']);
throw_rune_error('$derived', ['by']); throw_rune_error('$derived', ['by']);
throw_rune_error('$inspect'); throw_rune_error('$inspect');
throw_rune_error('$props', ['bindable']); throw_rune_error('$props');
throw_rune_error('$bindable');
} }
/** /**

Loading…
Cancel
Save