feat(compiler): detect unimported decorators #681
Merged
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
apapko
changed the title
apapko
force-pushed
the
apapko/unused-decorator-visitor
branch
from
e2c9a42
to
648b5b9
Compare
Benchmark resultsBase commit: |
2 similar comments
Benchmark resultsBase commit: |
Benchmark resultsBase commit: |
caridy
approved these changes
Sep 27, 2018
apapko
changed the title
Benchmark resultsBase commit: |
apapko
force-pushed
the
apapko/unused-decorator-visitor
branch
from
9291d72
to
648b5b9
Compare
pmdartus
reviewed
Sep 27, 2018
| @@ -26,8 +28,22 @@ module.exports = function LwcClassTransform(api) { | |||
| { | |||
| Program: { | |||
| exit(path, state) { | |||
| const visitors = mergeVisitors([ | |||
There was a problem hiding this comment.
Let's move the logic out of the file, we should not pollute the entry point.
| const visitors = mergeVisitors([ | ||
| classProperty(api, { loose: true }).visitor, | ||
| { | ||
| Decorator(path) { |
There was a problem hiding this comment.
You should add a comment here to explain why the Decorator visitor works here.
| } | ||
| }); | ||
|
|
||
| pluginTest('compiler should throw when "track" decorator is used in the inner class but was not imported from lwc', ` |
There was a problem hiding this comment.
that was testing the decorator usage with inheritance, but with our new logic to check decorators on exit this no longer is needed.
apapko
force-pushed
the
apapko/unused-decorator-visitor
branch
from
648b5b9
to
5ee44d2
Compare
Benchmark resultsBase commit: lwc-engine-benchmark
|
Benchmark resultsBase commit: lwc-engine-benchmark
|
pmdartus
approved these changes
Sep 27, 2018
Benchmark resultsBase commit: lwc-engine-benchmark
|
Benchmark resultsBase commit: |
Benchmark resultsBase commit: lwc-engine-benchmark
|
diervo
approved these changes
Oct 1, 2018
kevinv11n
reviewed
Oct 1, 2018
| } | ||
| `, { | ||
| error: { | ||
| message: 'Invalid decorator usage. Supported decorators (api, wire, track) should be imported from "lwc"', |
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
If babel-plugin-transform-lwc-class transform doesn't do early detection of a decorator usage that has not been imported from lwc, the issue is then caught by the babel with a bogus message. This PR adds validation earlier at the program visitor.
Fixes #601
Does this PR introduce a breaking change?