Skip to content

docs: mark protected fields internal#1015

Closed
spliffone wants to merge 1 commit intomainfrom
docs/mart-protected-fields-internal
Closed

docs: mark protected fields internal#1015
spliffone wants to merge 1 commit intomainfrom
docs/mart-protected-fields-internal

Conversation

@spliffone
Copy link
Copy Markdown
Member

Describe in detail what your merge request does and why. Add relevant
screenshots and reference related issues via Closes #XY or Related to #XY.


@spliffone spliffone requested a review from a team as a code owner November 12, 2025 12:31
@spliffone
Copy link
Copy Markdown
Member Author

@spike-rabbit According your previous comment I quickly added the internal flag to some protected members. I don't think we should go into this direction, it is just blowing up the file size without a real value. What I suggest instead, we adjust the api script to add a post-processing which drop the protected fields from the API. So the maintenance effort is zero compared to tagging all the fields.
From my point of view, protected members should never be used neither should consumers derive from our components/directives.

@github-actions
Copy link
Copy Markdown

Code Coverage

@github-actions
Copy link
Copy Markdown

@dr-itz
Copy link
Copy Markdown
Member

dr-itz commented Nov 27, 2025

Completely agree with @spliffone. adding @internal to protected is the wrong approach. And it only kills DX. We do protected exactly because it's not part of the public API.

IOW protected must not be part of the goldens.

@spike-rabbit
Copy link
Copy Markdown
Member

Then please find a way doing this. tools/test_api_report.ts may gives a hint how to abuse api-extractor internals.

Defining that protected equals internal is a SiMPL invention, that has nothing to do with standards. So this is obviously not supported by api-extractor.

Please also explain how @internal kills DX.
We could exclude internal fields from our emitted d.ts files using stripInternal ts compiler option.
This is currently not used, as we share a few internal fields between entry points. But we could fix this I guess.

@dr-itz
Copy link
Copy Markdown
Member

dr-itz commented Dec 23, 2025

Please also explain how @internal kills DX.

It adds unnecessary comment lines stretching the code, making it less easy to read. Waste of vertical space.
And also every commit that changes formatting (hello prettier) or adding stuff like @internal only adds stuff to the history of making it harder to trace things, i.e. when hunting bugs/figuring out why something is the way it is.

And it really should be the other way around. Everything that is added has to add value.

For me, in the current state, the API goldens do not add any value! It's just a step more. As long as it contains any Angular stuff (also the ng internal functions like ɵcmp, ɵfac, ɵprov). I.e. during review if I have to ignore most of the changes to the goldens and look at the public API changes vs. just look at the public API changes in the code, the added value is exactly zero. It only costs me more time.

@siemens/siemens-element-members

@spike-rabbit
Copy link
Copy Markdown
Member

I found a way to exclude protected easily in the meantime.

The goldens are anyway more about reviewer experience and maybe used to widen the code owners even more.
So this PR is no longer needed.

@dr-itz
Copy link
Copy Markdown
Member

dr-itz commented Dec 27, 2025

I found a way to exclude protected easily in the meantime.

The goldens are anyway more about reviewer experience and maybe used to widen the code owners even more. So this PR is no longer needed.

Unfortunately, #1237 is incomplete and buggy (i.e. it drops protected of non-Angular class). See #1246 for a better version

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.

3 participants