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: fixed for:each error on hover #491

Merged
merged 7 commits into from
Apr 4, 2022
Merged

fix: fixed for:each error on hover #491

merged 7 commits into from
Apr 4, 2022

Conversation

randi274
Copy link
Contributor

What does this PR do?

Addressed an issue where a for:each was not available, and was causing an undefined error.

To reproduce, hover over an item that doesn't have associated information with it, like the value of a for:each or a class name.

What issues does this PR fix or reference?

@W-10857157@, vscode #3929

Addressed an issue where a for:each was not available, and was causing an undefined error.

vscode #3929
@randi274 randi274 requested a review from a team as a code owner March 22, 2022 22:15
this.indexer.customData.forEach(t => {
t.classMembers.forEach(cm => {
this.indexer.customData?.forEach(t => {
t.classMembers?.forEach(cm => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we add a unit test for this condition? One to cover where customData isn't defined and one for when t.classMembers isn't defined?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I knew you were gonna ask for it 😆

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a unit test for the case when t.classMembers is undefined.

For indexer.customData, indexer.customData() is a wrapper around ComponentIndexer.tags, and tags can never be null (in the body of the class, it's defined as readonly tags: Map<string, Tag> = new Map();).

@@ -116,7 +116,7 @@ it('transform throws exceptions on syntax errors', async () => {
// verify err has the info we need
const message = extractMessageFromBabelError(err.message);
expect(message).toMatch('Unexpected token (4:17)');
expect(err.location).toEqual({ line: 4, column: 17 });
expect(err.location).toEqual({ line: 4, column: 17, index: 110 });
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why this was failing, but the addition of the index is now expected here.

@randi274 randi274 merged commit 6e8739d into main Apr 4, 2022
@randi274 randi274 deleted the randi/fix-hover-error branch April 4, 2022 19:41
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