Skip to content
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

feat(transformers): Add beforeDeclarations transformers #58879

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

Danielku15
Copy link

@Danielku15 Danielku15 commented Jun 16, 2024

Fixes #58880

Relates to #17146 and #41486 (not fully fixes them)

The custom transformers were lacking a beforeDeclarations allowing devs to do a AST transformation before the built-in transformers. If you wanted to use information from erased AST nodes (visibility, initializers, etc.) you were lost. With this change devs can improve their JSDoc comments or use other transformations specific for declarations.

Sorry for the PR without Backlog milestone issue. I still hope this makes it through.

@typescript-bot typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Jun 16, 2024
@typescript-bot
Copy link
Collaborator

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

@typescript-bot
Copy link
Collaborator

Looks like you're introducing a change to the public API surface area. If this includes breaking changes, please document them on our wiki's API Breaking Changes page.

Also, please make sure @DanielRosenwasser and @RyanCavanaugh are aware of the changes, just as a heads up.

@sandersn
Copy link
Member

To help with PR housekeeping, I'd prefer not to have PRs for bugs that aren't accepted yet. Discussion usually brings up new implementation requirements, and the codebase can change underneath the PR.

@sandersn sandersn self-assigned this Jun 18, 2024
@typescript-bot
Copy link
Collaborator

The TypeScript team hasn't accepted the linked issue #58880. If you can get it accepted, this PR will have a better chance of being reviewed.

@sandersn sandersn self-requested a review June 18, 2024 22:29
@Danielku15
Copy link
Author

Danielku15 commented Jun 19, 2024

@sandersn Typically I would fully agree that discussion should happen before PRs are opened and features proposed. I still hope that this would be a low-hanging-fruit topic we can easily pick. IMHO it was an oversight to properly add "beforeDeclarations" when "afterDeclarations" transformers were added (considering normal before/after exists too). I really hope that not the whole functionality as such (pre-existing) is being challenged as part of an extension where 3/4 combinations are enhanced to ship 4/4.

As discussion around such topics often happens across years, and is spread across thousands of issues. It is hard to expect people will become active on a new issue. With this I hope the maintainers dive at least a bit into the issue and PR to reflect on it. Its really not such a big deal with direct added benefit. Also considering there is already a tree of partially related issues with interest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Uncommitted Bug PR for untriaged, rejected, closed or missing bug
Projects
Status: Waiting on author
Development

Successfully merging this pull request may close these issues.

Add support for beforeDeclarations
3 participants