Fix array-style props corrupting data(), and props being dropped from compiled output - #9
Merged
Merged
Conversation
…ps being silently dropped entirely
Continued the @teloce/sfc audit, found two compounding bugs affecting
`props` declarations - both confirmed with real compile tests.
extractObjectProperty jumping to the wrong `{` entirely (packages/sfc/
src/script/index.ts):
- After finding a top-level key via findTopLevelKey, this searched for
the property's value with `objStr.indexOf('{', keyIdx)` - the
position of the *nearest* `{` anywhere after the key, with no check
that the key's own value was actually an object literal in the first
place. For an array-style props declaration (`props: ['label',
'count']` - a real, documented, valid way to declare props with no
object literal anywhere in it), that search skipped straight past the
array and matched a completely unrelated *later* property's opening
brace. Confirmed via an actual compile: `props: ['label', 'count'],
data() { return {}; }` extracted data()'s own function body content
and assigned it as if it were the props definition - silently
corrupting one property's extracted value with a different property's
content entirely, with zero error reported. Fixed by verifying the
character immediately following `propName:` (skipping whitespace) is
actually `{` before searching for it at all; otherwise this correctly
returns null so the array-form fallback (already present, from an
earlier commit's fix) gets a chance to run instead.
props silently dropped from the final compiled output regardless
(packages/sfc/src/compile.ts):
- Independent of the above, generateCode() never emitted `props` into
the compiled defineComponent({...}) call at all - only data/methods/
computed/lifecycle. A component author writing `props: [...]` or the
object form had it correctly extracted by compileScript but then
discarded outright, so the compiled component definition never
carried a props field regardless of what was written. (This didn't
break the component-composition prop-passing added in an earlier
commit, since that overlays whatever a parent passes directly into
the child's reactive state without consulting the child's own
declared props list at all - but it meant `props: [...]` as written
by a component author was silently dead syntax with zero effect,
which is exactly the kind of "wrote something reasonable, nothing
happened, no error" gap this whole audit has been finding.) Fixed by
adding `props: ${exports.props}` to the generated output alongside
the other fields.
Verified: array-form and object-form props now both correctly appear in
compiled output, with data()/methods no longer corrupted by the fix (
re-verified the earlier nested-methods-collision fix from a previous
commit still holds); full regression sweep across everything from this
session and previous ones still passes; full "pnpm -r build" across the
whole workspace with zero errors.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
One commit that landed after PR #8 was already merged - see commit message for full details. Fixes two compounding bugs: array-form props (
props: [...]) could silently corrupt an unrelated later property (usually data()), and props were never emitted into the compiled output at all regardless of form.