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

fix: Rename identifier for source map #1419

Closed

Conversation

liuxingbaoyu
Copy link
Contributor

@liuxingbaoyu liuxingbaoyu commented Oct 7, 2022

I found that computeColumnSpans can be used to get a more accurate identifier.
I only did a simple test locally, but this should theoretically be fine.

Fixed: #1383

@liuxingbaoyu
Copy link
Contributor Author

liuxingbaoyu commented Oct 7, 2022

I don't understand what's going on with CI, is this related to PR?

@connor4312
Copy link
Member

Thanks for the PR. I will get to this in a little bit--I'm looking at switching to a new, mostly compatible sourcemap library which at minimum will cause some merge conflicts here, but I will get it merged.

@connor4312
Copy link
Member

Thanks again, I extended this in #1481 to avoid computing the whole set of column spans and just using the next mapping as we iterate (IeachMapping ensures order)

@connor4312 connor4312 closed this Dec 14, 2022
@liuxingbaoyu
Copy link
Contributor Author

Sorry, there seems to be an issue with this PR. (may block publishing)
It seems that some generator sourcemaps may have inaccurate identifier ranges.
For example (aaaa), some generators may generate aaaa) as an item.
I fear this PR will make this worse.
Maybe we can introduce a detection to allow fallback, we use regex to detect all text in the identifier range, if it contains ,),,, then let us fall back to using regex logic. (maybe allow .)
Also I realize there's a possible problem with the new logic treating member expressions as allowing previews, which has the slightest chance of side effects (getters), but that's probably fine since we're using the location in the source code to look for, which means that only compiler-generated member expressions are previewed.

@connor4312
Copy link
Member

hm, thanks. For now I think I will just go back to the original approach, and modify the regex to allow member access with .

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.

Unable to preview named import for ts with sourcemap
2 participants