feat(compiler): More tooling metadata #132
Conversation
Benchmark comparisonBase commit:
|
rsalvador
force-pushed
the
rsalvador/metadata
branch
from
530e229 to
acd72a2
Compare
Benchmark comparisonBase commit:
|
| { | ||
| output: { | ||
| metadata: { | ||
| decorators: [ |
There was a problem hiding this comment.
classMembers section now seems to be the superSet of what 'decorator' section provided. @yungcheng do you think we should just delete the 'decorators' section and have consumers derive whatever they need from classMembers section or do you think that its needed to reduce processing overhead on consumer end? I understand that you'll need to move some of the extra metadata collected for @wire over for this but it may be good for consolidation.
There was a problem hiding this comment.
I prefer to have it separated because compiler already visited all decorators, why would we want consumers to do the similar thing again on class members? Also the usage is quite different, class member provides general information for intellisense, decorators are used for dependencies. However if we end up tightening things up with platform compiler and have all information in class member, then I am okay with deleting decorators since you would need to process class member on the server.
| return { | ||
| Class(path, state) { | ||
| if (isComponentClass(path, state.componentBaseClassImports)) { | ||
| if (isDefaultExport(path)) { |
There was a problem hiding this comment.
combine both check maybe?
|
|
||
| state.file.metadata.classMembers = []; | ||
| const classBody = path.get('body'); | ||
| classBody.get('body').forEach(path => { |
There was a problem hiding this comment.
classBody = path.get('body').get('body') maybe? seems to make more sense
rsalvador
force-pushed
the
rsalvador/metadata
branch
from
acd72a2 to
2f06bed
Compare
Benchmark comparisonBase commit:
|
|
I was debating with @pmdartus and maybe we want to move all of the metadata gathering to the Platform compiler instead? We don't have to do it in 214, but as we add more metadata (and the shape will likely change) we are not sure where things will live... Food for tought. |
|
We’ll need that metadata for the public vscode extension. |
| removeDecorators(decorators); | ||
| removeImportSpecifiers(decoratorImportSpecifiers); | ||
| Class(path, state) { | ||
| // don't remove decorators until metadata.js had the chance to visit the Class node |
There was a problem hiding this comment.
Eventually it would be good to split the compiler into 2 separate phases:
- validation + metadata collection
- AST transformation
rsalvador
force-pushed
the
rsalvador/metadata
branch
from
2f06bed to
6927956
Compare
Benchmark comparisonBase commit:
|
Details
For tooling we need metadata location/doc info for all class properties and methods
Does this PR introduce a breaking change?
If yes, please describe the impact and migration path for existing applications:
Please check if your PR fulfills the following requirements:
Reminders ( please delete this section before submitting )
The PR fulfills these requirements:
PR Title
LWC PR title follows conventional commit format and is automatically validated by our CI.