We read every piece of feedback, and take your input very seriously.
To see all available qualifiers, see our documentation.
There was an error while loading. Please reload this page.
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
They should work exactly as they do in typescript.
Fixes #14009
This adds a potentially expensive check when retrieving modifiers, but it should only apply to JS, and modifiers are usually cached, so I don't think the impact will be that great.
In any case it should be completely unobservable from our current performance suite. I'll double-check that.
Sorry, something went wrong.
Add @private/@protected/@public test
87666a9
Fix @declaration
9c1a370
draft abstraction + one usage
88dcb39
Fill in necessary parsing etc
871a643
make general getEffectiveModifierFlags
455b7b3
move to utilities, make the right things call it
reorder public/private/protected
bd5781a
JS declaration emit works with @public/@private/@Protected
14d3c38
revert unneeded/incorrect changes
b80847b
Update baselines and skip @public/etc when parsing
62a568d
1. Update the API baselines with the new functions. 2. Do not check for @public/etc during parsing, because parent pointers aren't set, so non-local tags will be missed; this wrong answer will then be cached.
There was a problem hiding this comment.
The reason will be displayed to describe this comment to others. Learn more.
this is the only substantive change in this file; all the others just eliminate checks that are already performed in ensureType.
ensureType
@typescript-bot perf test this
Heya here. It should now contribute to this PR's status checks.
Update: The results are in!
@sandersn The results of the perf run you requested are in!
Comparison Report - master..35731
Shouldn't you just integrate this into getModifierFlagsNoCache? (Thus simultaneously caching the result and preventing the need for a separate getEffectiveModifierFlags function) It already has special logic for isInJSDocNamespace stuff, so...
getModifierFlagsNoCache
getEffectiveModifierFlags
isInJSDocNamespace
Parser: don't call hasModifier(s) anymore.
f30f399
Then move jsdoc modifier tag checks into getModifierFlagsNoCache where they should be. The jsdoc checks are skipped when the parent is undefined. There are 3 cases when this can happen: 1. The code is in the parser (or a few places in the binder, notably declareSymbol of module.exports assignments). 2. The node is a source file. 3. The node is synthetic, which I assume to be from the transforms. It is fine to call getModifierFlags in cases (2) and (3). It is not fine for (1), so I removed these calls and replaced them with simple iteration over the modifiers. Worth noting: Ron's uniform node construction PR removes these calls anyway; this PR just does it early.
OK, the hasModifier-less parser code is ready. I expect this version will have no observable performance difference, so ...
hasModifier
Merge branch 'master' into private/protected-jsdoc
37a586a
Yep, there is now no perf change from this PR.
Reminder: We should still also add support for interpreting #17233
This is a parse error in the playground. This test isn't flagging it because the input code has errors, so output validity isn't checked. You should split erroring/non-erroring cases into separate tests.
Oh, I'd gotten so used to errors and symbol baselines co-existing I forgot that errors and declarations didn't. I'll create a separate declaration emit test.
Fix constructor emit
8182bfd
1. Emit protected for protected, which I missed earlier. 2. Emit a constructor, not a property named "constructor". 3. Split declaration emit tests out so that errors are properly reported there.
All right, declaration emit, and its tests, are fixed now.
3c5ecc2
Not clear what to do about type-space references to readonly, like
/** @typedef {Object} x * @readonly @property {number} foo */
But that's not in the jsdoc tool and not supported by closure as far as I know, so it's fine not to support it.
How does this handle cases like this? What takes precedence?
/** @private */ public getFoo(): string { /* ... */ }
Background: I've been using this for documentation generation, for members that I'm using internally (in other subpackages of the same monorepo), but that shouldn't be included in the documentation.
@d-fischer in TS files Jsdoc does not affect types. In JS files you can't use the public modifier so it cannot conflict with @private
public
@private
@d-fischer in TS files Jsdoc does not affect types.
That's all I needed to know, thanks!
weswigham weswigham left review comments
andrewbranch andrewbranch approved these changes
sandersn
Successfully merging this pull request may close these issues.