Conversation
text/0000-private-methods.md
Outdated
| This approach delegates to native browser behavior of private methods and is a backwards compatible solution. [You can find a POC PR here.](https://github.com/salesforce/lwc/pull/5477) | ||
|
|
||
| **Section of Random Facts Relating to the Design** | ||
| * We do not need to transform call expressions within a component, this is already functional |
There was a problem hiding this comment.
I didn't get this part, can you explain?
There was a problem hiding this comment.
I'm guessing that the current compiler can handle call expressions (this.#privateMethod()), and it only breaks on method declarations (#privateMethod() { ... }).
There was a problem hiding this comment.
Feels yucky to have an intermediate compiled state where the javascript is intentionally broken...just me?
There was a problem hiding this comment.
@ekashida Do you mean when intermediate moment when the method declaration is _internal_only_private_privateMethodCall() but the call expression is still this.#privateMethod()?
There was a problem hiding this comment.
There is definitely merit to that "yucky" feeling. The primary concern is the creation of an
invalid intermediate AST where the declaration and invocation are temporarily out of sync.
This leads to a few specific risks:
Plugin Conflicts: If a linter, minifier, or tree-shaker runs during that intermediate "window,"
it may see a call to #privateMethod with no corresponding declaration and either crash or
produce incorrect optimizations.
Shadowing & Collisions: Transforming to a standard string like _internal_only_... risks
shadowing existing properties or global variables. An intermediate transform might
misinterpret the intent of the renamed method and "optimize" it in a way that breaks the
link to the original source.
Technical Debt in the Pipeline: If the transforms aren't strictly atomic and encapsulated,
the compiler is essentially "lying" to the rest of the pipeline about the class structure,
making the build process fragile and difficult to debug when stack traces show "fake"
method names.
There was a problem hiding this comment.
These points are pretty convincing. Let's ensure that the output of the transform maintains valid structure? We could collect all the private method names when we enter ClassBody, and then use those in the visitors for ClassPrivateMethod and MemberExpression for mangling.
e0971e9 to
0575ab3
Compare
|
@caridy @ravijayaramappa can you take a look at this? |
| #privateMethod() { ... } | ||
| ``` | ||
|
|
||
| Private methods can be referenced using dot notation: |
There was a problem hiding this comment.
To highlight the utility of private methods, it might be good to include the rest of the class context, and to contrast that with an example of disallowed syntax (like (arg) => arg.#privateMethod()).
| * This feature is limited to implementing only private methods. It does not include not private properties or private accessors (getter/setters). | ||
| * Both of these transforms only get applied if enabled, using a compiler option | ||
|
|
||
| Private class members can only be accessed from within the class body itself, as defined by JavaScript's private fields specification. You cannot call `instance.#privateMethod()` from external code because it attempts to access the private method from outside the class context. Similarly, referencing `{#privateMethod}` in an LWC template violates this same encapsulation rule because the template exists outside the JavaScript class body. Both result in a SyntaxError caught before the code ever runs. |
There was a problem hiding this comment.
What are the existing errors for instance.#privateMethod() and {#privateMethod}. Particularly for the latter, should we change them to provide more clear guidance?
There was a problem hiding this comment.
The first one is completely self-explanatory but it's a typescript error. The javascript error ends up being the same as the second example: SyntaxError: Private field '#bar' must be declared in an enclosing class.
Not as user-friendly, but it's a native syntax error that is easily searchable. Assuming it would be simple to hook into the exception and provide an error message similar to the typescript error, but I'm ok with keeping it as-is.
text/0000-private-methods.md
Outdated
|
|
||
| ## Drawbacks | ||
| **Drawback #1:** Performance. | ||
| This design requires AST traversal twice, once to rename the private functions to regular functions & then again to re-rename them as private functions. There is performance overhead to this AST traversal. |
There was a problem hiding this comment.
🤔 Does it require separate traversals or just an extra plugin call within the single traversal?
There was a problem hiding this comment.
As implemented in the POC PR, there is an extra manual traversal of the tree for the Program visitor.
ravijayaramappa
left a comment
There was a problem hiding this comment.
Overall seems like a good move. Had some questions about the implementation strategy.
|
|
||
| **Drawback #2:** Complexity to LWC Compiler. | ||
| As with all expansions, this project adds complexity and technical debt to the LWC compiler. This project is implementing two new babel transforms that will need to be maintained. | ||
|
|
There was a problem hiding this comment.
I assume the transformation will apply to other classes that a component might have, for example quill library. Should we account for that risk exposure.
There was a problem hiding this comment.
The transformation is really a two-step non-transformation. Private method in => private method out. If our transformations are #methodName => __lwc__private_method_methodName => #methodName, then it should be fairly low risk. The bigger risk is probably how our intermediate stage might interact with other babel plugins.
text/0000-private-methods.md
Outdated
| ``` | ||
|
|
||
| ## Motivation | ||
| The largest motivation for this feature is improving our security approach. Enabling private methods helps maintain the expected behavior of a component by restricting access to its internal methods. This will prevent unexpected context abuse and shadowing methods that alter the behavior of a component. Private methods are inherently secure by design, and using them eliminates entire classes of potential component vulnerabilities. |
There was a problem hiding this comment.
unexpected context abuse and shadowing methods
- How much of a problem is this? Since this RFC is restricted to methods only.
- What's our stance for customers (internally & externally) who have taken a dependency on existing behavior? It could be
we will follow this for net new components but cannot break existing exposed components.
There was a problem hiding this comment.
How much of a problem is this? Since this RFC is restricted to methods only.
We don't know the scope of the problem, but it will be gated with a flag within the component.
How much of a problem is this? Since this RFC is restricted to methods only.
What's our stance for customers (internally & externally) who have taken a dependency on existing behavior? It could be we will follow this for net new components but cannot break existing exposed components.
This will likely be a case-by-case basis. The guidance within base components will be exactly that--use private methods for net-new, but gate usage in existing. We didn't include this in the RFC since it's not directly relevant to the feature.
text/0000-private-methods.md
Outdated
|
|
||
| ## Drawbacks | ||
| **Drawback #1:** Performance. | ||
| This design requires AST traversal twice, once to rename the private functions to regular functions & then again to re-rename them as private functions. There is performance overhead to this AST traversal. |
There was a problem hiding this comment.
As implemented in the POC PR, there is an extra manual traversal of the tree for the Program visitor.
text/0000-private-methods.md
Outdated
|
|
||
| As you can see in the screenshot above, the linter suggests to “Please add @babel/plugin-transform-private methods to your configuration” when using private methods in an LWC. This Babel transform lets you use JavaScript private methods in environments that don’t support them yet by transforming them into older, compatible JavaScript. In our use case, the Babel transform would perform compile time validation on private properties (anything without LWC decorators, such as @wire, @api, and @track). | ||
|
|
||
| This design causes issues because class properties get renamed, thus becoming unusable in the template. While this solution does remove the error, it does not resolve the security issues within the components. It is also adding a polyfill for something that is already natively supported in browsers. |
|
|
||
| The design spelled out above is backwards compatible and does not break existing LWC code. It will be adopted internally before it is enabled and exposed for external customers. | ||
|
|
||
| A simple LWC example could be written for demonstration purposes. |
There was a problem hiding this comment.
Sounds like a TODO :)
| **Drawback #2:** Complexity to LWC Compiler. | ||
| As with all expansions, this project adds complexity and technical debt to the LWC compiler. This project is implementing two new babel transforms that will need to be maintained. | ||
|
|
||
| ## Alternatives |
There was a problem hiding this comment.
Question: Why does @babel/plugin-transform-class-properties block private methods? seems arbitrary that a plugin that only transforms class "fields" complains when it sees a private "method". If the reason does not actually affect us, can we fork @babel/plugin-transform-class-properties and modify for our needs?
There was a problem hiding this comment.
I guess it's a DX feature going off the assumption that you're using the plugin because your client doesn't support private features, and it doesn't make sense to transform private fields but not private methods?
There was a problem hiding this comment.
Babel aligns its plugins with proposed language features. Class properties and private fields were separate proposals, so they were created as separate plugins.
| **Drawback #2:** Complexity to LWC Compiler. | ||
| As with all expansions, this project adds complexity and technical debt to the LWC compiler. This project is implementing two new babel transforms that will need to be maintained. | ||
|
|
||
| ## Alternatives |
There was a problem hiding this comment.
I haven’t evaluated the trade-offs or risks for this, just thinking out loud
can we use decorators with accessors, to implement reactivity? That is the compiler adds accessor keyword and a decorator on all class fields, and the engine defines that decorator to observe for changes and re-render when needed.
Babel has a plugin for transpilation until we get native support
There was a problem hiding this comment.
Decorators were a bet that didn't pay off, and we've seen from other features (like synthetic shadow) that relying on non-native implementations of things is just a headache down the road.



RFC: Private Methods in LWC