Skip to content

Commit

Permalink
fix: tighten up export default validation
Browse files Browse the repository at this point in the history
through #14363 I noticed our `export default` validation wasn't quite right:
- missed checking for derived/state exports
- the "cannot have a default export" error was only thrown if you did `export default` from the instance script, but it shouldn't matter from which component part you export it; it's never ok
  • Loading branch information
dummdidumm committed Nov 20, 2024
1 parent 32a1453 commit a62f408
Show file tree
Hide file tree
Showing 10 changed files with 83 additions and 26 deletions.
5 changes: 5 additions & 0 deletions .changeset/new-houses-roll.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'svelte': patch
---

fix: tighten up `export default` validation
Original file line number Diff line number Diff line change
@@ -1,13 +1,18 @@
/** @import { ExportDefaultDeclaration, Node } from 'estree' */
/** @import { ExportDefaultDeclaration } from 'estree' */
/** @import { Context } from '../types' */
import * as e from '../../../errors.js';
import { validate_export } from './shared/utils.js';

/**
* @param {ExportDefaultDeclaration} node
* @param {Context} context
*/
export function ExportDefaultDeclaration(node, context) {
if (context.state.ast_type === 'instance') {
if (!context.state.ast_type /* .svelte.js module */) {
if (node.declaration.type === 'Identifier') {
validate_export(node, context.state.scope, node.declaration.name);
}
} else {
e.module_illegal_default_export(node);
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,8 +1,6 @@
/** @import { ExportSpecifier, Node } from 'estree' */
/** @import { Binding } from '#compiler' */
/** @import { ExportSpecifier } from 'estree' */
/** @import { Context } from '../types' */
/** @import { Scope } from '../../scope' */
import * as e from '../../../errors.js';
import { validate_export } from './shared/utils.js';

/**
* @param {ExportSpecifier} node
Expand Down Expand Up @@ -30,22 +28,3 @@ export function ExportSpecifier(node, context) {
validate_export(node, context.state.scope, local_name);
}
}

/**
*
* @param {Node} node
* @param {Scope} scope
* @param {string} name
*/
function validate_export(node, scope, name) {
const binding = scope.get(name);
if (!binding) return;

if (binding.kind === 'derived') {
e.derived_invalid_export(node);
}

if ((binding.kind === 'state' || binding.kind === 'raw_state') && binding.reassigned) {
e.state_invalid_export(node);
}
}
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/** @import { AssignmentExpression, Expression, Literal, Pattern, PrivateIdentifier, Super, UpdateExpression, VariableDeclarator } from 'estree' */
/** @import { AssignmentExpression, Expression, Literal, Node, Pattern, PrivateIdentifier, Super, UpdateExpression, VariableDeclarator } from 'estree' */
/** @import { AST, Binding } from '#compiler' */
/** @import { AnalysisState, Context } from '../../types' */
/** @import { Scope } from '../../../scope' */
Expand Down Expand Up @@ -263,3 +263,22 @@ export function validate_identifier_name(binding, function_depth) {
}
}
}

/**
* Checks that the exported name is not a derived or reassigned state variable.
* @param {Node} node
* @param {Scope} scope
* @param {string} name
*/
export function validate_export(node, scope, name) {
const binding = scope.get(name);
if (!binding) return;

if (binding.kind === 'derived') {
e.derived_invalid_export(node);
}

if ((binding.kind === 'state' || binding.kind === 'raw_state') && binding.reassigned) {
e.state_invalid_export(node);
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { test } from '../../test';

export default test({
error: {
code: 'derived_invalid_export',
message:
'Cannot export derived state from a module. To expose the current derived value, export a function returning its value',
position: [61, 83]
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
let count = $state(0);

const double = $derived(count * 2);

export default double;
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import { test } from '../../test';

export default test({
error: {
code: 'state_invalid_export',
message:
"Cannot export state from a module if it is reassigned. Either export a function returning the state value or only mutate the state value's properties",
position: [93, 118]
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
let primitive = $state('nope');

export function update_primitive() {
primitive = 'yep';
}

export default primitive;
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
[
{
"code": "module_illegal_default_export",
"message": "A component cannot have a default export",
"start": {
"line": 2,
"column": 1
},
"end": {
"line": 2,
"column": 19
}
}
]
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
<script module>
export default 42;
</script>

0 comments on commit a62f408

Please sign in to comment.