feat(compiler): collect default value metadata for @api properties #816
Conversation
This file contains 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
Benchmark resultsBase commit: |
Benchmark resultsBase commit: lwc-engine-benchmark
|
ghost
reviewed
Nov 9, 2018
| ` | ||
| import { LightningElement, api } from 'lwc'; | ||
| export default class Foo extends LightningElement { | ||
| _privateTodo |
ghost
reviewed
Nov 9, 2018
| type: "property", | ||
| value: { | ||
| type: "object", | ||
| value: { |
ghost
reviewed
Nov 9, 2018
| { type: "string", value: "stringval" }, | ||
| { type: "number", value: 0 }, | ||
| { type: "null", value: null }, | ||
| { type: undefined, value: undefined }, |
There was a problem hiding this comment.
I wonder if type should be undefined vs "undefined", as for example null technically is also an undefined type... just something to ponder about
There was a problem hiding this comment.
my thinking was as follows:
@api nullProp = null;since the null has its own type - NullLiteral, we can capture it explicitly as a 'null' type;
Versus absence of value,
@api novalue;where we cannot deduce its type, in which case it is truly undefined.
Anyhow, i will be adding 'unresolved' for consistency
ghost
reviewed
Nov 9, 2018
| } | ||
|
|
||
| if (type === 'property') { | ||
| meta.value = extractValueMetadata(parentValue) |
ghost
reviewed
Nov 9, 2018
| function extractStringValueMeta(valueNode) { | ||
| return { | ||
| type: 'string', | ||
| value: valueNode && valueNode.value || undefined, |
There was a problem hiding this comment.
when would the undefined condition kick in ?
There was a problem hiding this comment.
absence of value in valueNode.value is returned as 'null'; therefore returning undefined instead.
ghost
reviewed
Nov 9, 2018
| } | ||
|
|
||
| if (!valueNode || !Array.isArray(valueNode.elements)) { | ||
| return arrayValueMeta; |
There was a problem hiding this comment.
elements always have value of empty array, i'm just being paranoid :). Will remove the !Array.isArray(valueNode.elements) check.
ghost
reviewed
Nov 9, 2018
|
|
||
| const values = {}; | ||
|
|
||
| // TODO: do we want this as an object with properties mapped to the value objects |
There was a problem hiding this comment.
probably a map (as is done here) is better, more in line with expectations
ghost
reviewed
Nov 9, 2018
| @@ -184,6 +184,93 @@ function generateError(source, { errorInfo, messageArgs } = {}) { | |||
| return error; | |||
| } | |||
|
|
|||
| function extractValueMetadata(valueNode) { | |||
|
|
|||
| // TODO: should we flag anything we can't resolve , such as expressions, as type: 'unresolved' instead of undefined? | |||
There was a problem hiding this comment.
that would probably be more consistent with the way wire decorator value metadata works, either way we do it, it's just best to keep it consistent types wise.
There was a problem hiding this comment.
What i'm currently doing is not collecting value metadata for expressions at all, since we cannot resolve their value; However, the absence of a default value on the static property is captured as undefined.
Benchmark resultsBase commit: lwc-engine-benchmark
|
Benchmark resultsBase commit: lwc-engine-benchmark
|
Benchmark resultsBase commit: lwc-engine-benchmark
|
diervo
force-pushed
the
apapko/meta-prop-value
branch
from
128a380
to
473f615
Compare
Benchmark resultsBase commit: lwc-engine-benchmark
|
apapko
force-pushed
the
apapko/meta-prop-value
branch
from
473f615
to
aaa86e1
Compare
Benchmark resultsBase commit: lwc-engine-benchmark
|
Benchmark resultsBase commit: lwc-engine-benchmark
|
Benchmark resultsBase commit: lwc-engine-benchmark
|
apapko
force-pushed
the
apapko/meta-prop-value
branch
from
fcdb8e5
to
b051f42
Compare
apapko
force-pushed
the
apapko/meta-prop-value
branch
from
b051f42
to
3f8876b
Compare
Benchmark resultsBase commit: lwc-engine-benchmark
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Details
Today, metadata object produced by the compiler does not contain default values for public properties.
In order for the component metadata consumers to detect default values of the public properties, the metadata object must send back the value object for both classMember and decorators metadata objects ( api properties only as it is the only public property, and no methods )
ex:
will produce :
Does this PR introduce a breaking change?
Any tests in lwc-platform and aura that validate metadata shape may need to be edited.