Skip to content

Fix built-in property visibility bug cause by LwcFlavor #229

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 16, 2021

Conversation

justinfagnani
Copy link
Collaborator

@justinfagnani justinfagnani commented Sep 15, 2021

This was a rough one to figure out.

The way flavors work they all get a chance to work against a definition and potentially refine features. A bug in LwcFlavor caused it to do work again non-LWC definitions and part of its feature refinement is to default all class members to protected visibility. This refinement was running against all built-in definitions cause built-in public properties to be changed to protected. This was only caught by one security test in lit-analyzer because lit-analyzer filters out non-public fields before type-checking.

@justinfagnani justinfagnani changed the title Add failing test for built-in property visibility bug Fix built-in property visibility bug cause by LwcFlavor Sep 16, 2021
@justinfagnani
Copy link
Collaborator Author

Great. This is passing with npm test but failing with npm run test:all - apparently it can't find the built-in script element definition in that config.

@justinfagnani
Copy link
Collaborator Author

All tests passing now. @rictic

@justinfagnani justinfagnani merged commit abdc6e2 into master Sep 16, 2021
@justinfagnani justinfagnani deleted the property-visibility-bug branch September 16, 2021 18:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants