refactor glimmer post-process, better handle template tag #1795
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| // defined. the `token` should be the specific template API which the error is on, | ||
| // so we need to find the usage of the token in the template | ||
|
|
||
| // TODO: this is still bug-prone, and should be refactored to do something like: |
There was a problem hiding this comment.
I think we kinda have two options to making this less bug prone:
- re-work template-imports / glimmer-parse / getTemplateLocals to get us positions
- upstream
<template>to TC39, making our lives easier across the whole ecosystem. https://wycats.github.io/polaris-sketchwork/content-tag/second-phase-compiler.html
NullVoxPopuli
approved these changes
Mar 8, 2023
lin-ll
reviewed
Mar 8, 2023
Co-authored-by: Lucy Lin <lucy.ly.lin@gmail.com>
lin-ll
approved these changes
Mar 9, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
refactor postprocessing of template placeholders and re-map back to source more appropriately.
This PR makes many changes based on the assumption that the number of lines in the original code equals the number of lines in the transformed code. The code now throws an error if this assumption is not met. With this assumption in place, a lot of mapping from transformed -> original becomes a lot simpler: for a lint error on line X, when the original source content at line X matches the transformed source content at line X, we can return the original diagnostic message with no changes.
The remainder of the work in this PR is around mapping placeholder template values (
[__GLIMMER_TEMPLATEback into its original form of<template>, and adjusting thecolumnfield in the diagnostic to match the changes. Same goes for the "closing tag" placeholder.We still currently use the error prone "find token and loop through source" method when the diagnostic "token" is inside a template tag, but there is a failing test (commented out currently) which exhibits how this code is currently broken.
Other big change here is to generally add support for multiline lint errors, which never existed in the original implementation.