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

Event name is not parsed correct if fired with identifier instead string constant #1

Closed
3 tasks done
alexprey opened this issue Oct 30, 2018 · 11 comments · Fixed by #46 or #54
Closed
3 tasks done

Event name is not parsed correct if fired with identifier instead string constant #1

alexprey opened this issue Oct 30, 2018 · 11 comments · Fixed by #46 or #54
Labels
enhancement New feature or request Svelte V2 Issue related to Svelte V2 components Svelte V3 Issue related to Svelte V3 components
Milestone

Comments

@alexprey
Copy link
Collaborator

alexprey commented Oct 30, 2018

Event name is not parsed correct if fired with identifier instead string constant

import { createEventDispatcher } from 'svelte';
const dispatch = createEventDispatcher();

const CLOSE_EVENT_NAME = 'modalWasClosed';
dispatch(CLOSE_EVENT_NAME);

Actual result

{
    "events": [
        {
            "name": "****unhandled-event-name****"
        }
    ]
}

Expected result

{
    "events": [
        {
            "name": "click"
        }
    ]
}

UPD. At 4.0.0 point this issue are partially fixed.

  • Support the top-level plain constants (EVENT_MODAL_CLOSE)
  • Support the top-level object-level constants (EVENT.MODAL.CLOSE)
  • Support the top-level object-level constants with accessing by index getter (EVENT['MODAL']['CLOSE']). Actually, I'm not sure that this are required
@alexprey alexprey added the enhancement New feature or request label Nov 26, 2018
@alexprey
Copy link
Collaborator Author

The same issue for svelte3 syntax

import { createEventDispatcher } from 'svelte';
const dispatch = createEventDispatcher();

const CLOSE_EVENT_NAME = 'modalWasClosed';
dispatch(CLOSE_EVENT_NAME);

@alexprey alexprey added Svelte V2 Issue related to Svelte V2 components Svelte V3 Issue related to Svelte V3 components labels Aug 17, 2020
@TheComputerM
Copy link

Yeah, having the same problem with svelte 3

@TheComputerM
Copy link

If there is something like:

function dismiss() {
  visible = false;
  dispatch('dismiss', {
    visible,
  });
}

The event dismiss is properly shown in the doc, but if the function is exported:

export function dismiss() {
  visible = false;
  dispatch('dismiss', {
    visible,
  });
}

The event dismiss is not shown at all.

@alexprey
Copy link
Collaborator Author

@TheComputerM it looks like a new issue, I'm create a new one, thanks you for reporting!

soft-decay added a commit to soft-decay/sveltedoc-parser that referenced this issue Jan 17, 2021
- Add utils functions to help parse identifiers:
- Fix potential bugs:
  - utils.value was mutating the argument
  - parseEventDeclaration could attempt to read property of 'undefined'
- Add/Update tests (3) for event name parsing in parser v2 and v3
- Add unit tests (3) for new utils function
@soft-decay
Copy link
Contributor

soft-decay commented Jan 17, 2021

Hi @alexprey. I Hope you had good holidays.

This is a very old issue, but I saw it was also affecting the v3 parser, so I worked on it and opened a PR (#46) for a fix.
It allows basic support for identifier parsing of events:

  • can only recognize top level identifiers
  • nested identifiers in MemberExpression have to be dot notation only (computed with brackets not supported)
  • Supports only nested Literal and ObjectExpression

If you want to see support for more types of node, or if you think it should support bracket notation for identifier parsing, let me know. I also left some TODOs in the PR that I'm going to complete soon.

soft-decay added a commit to soft-decay/sveltedoc-parser that referenced this issue Jan 23, 2021
…atChaotic#1)

- Add util function isPunctuatorToken
- Improve documentation for utils functions:
- buildPropertyAccessorChainFromTokens
- buildPropertyAccessorChainFromAst
- getValueForPropertyAccessorChain
- Add unit tests (6) for the functions mentioned above
- Improve examples/alert: use identifer for event
alexprey added a commit that referenced this issue Jan 25, 2021
fix(parser): Parse event name when using an identifier (#1)
@alexprey
Copy link
Collaborator Author

At 4.0.0 point this issue are partially fixed. However to complete this issue we need more work. Not sure that supporting of left cases should be solved in the area of this library. For example - support of imported identifiers. Actually the library dont have any logic to resolve import files or analyze them. But if we can support that it will be nice.
I suppose that I can mark this isuue as low-priority, due we cover basic cases of identifier usage in event names.

@soft-decay
Copy link
Contributor

soft-decay commented Jan 25, 2021

The bracket notation support would be nice to have, but indeed it is not that important.

However, supporting imports parsing is a whole other story, and I feel like it should have its own separate issue. The way I see it, if the library supports the resolution and parsing of imported files, imported identifiers become top level identifiers, so the same logic can be applied to them.

What do you think about creating a new issue that is more specific to the new problem (Resolve imported identifiers)?

@alexprey
Copy link
Collaborator Author

You are right, the imported identifiers handling looks like a new story. Keep only bracets notation in this issue and create a new one to support external references

@alexprey
Copy link
Collaborator Author

@soft-decay I create a new issue to discuss the architecture for imported identifiers - #48. I mark it with low priority, due TypeScript support is more important for that.

soft-decay added a commit to soft-decay/sveltedoc-parser that referenced this issue Jan 27, 2021
…atChaotic#1)

- parser v3: bracket notation for string 'Literal' nodes
- parser v2: bracket notation for String tokens
- Add unit tests (2 per parser) for bracket notation support
@soft-decay
Copy link
Contributor

I added support for bracket notation with string literals in this PR #54. I did not implement nested computed Identifier. e.g. :

dispatch(EVENT[OTHER.CONSTANT].NOTIFY) // Not supported
dispatch(EVENT["NESTED"].NOTIFY) // Supported

@alexprey
Copy link
Collaborator Author

dispatch(EVENT[OTHER.CONSTANT].NOTIFY) // Not supported

I think that is out of scope, so ok

soft-decay added a commit to soft-decay/sveltedoc-parser that referenced this issue Jan 27, 2021
alexprey added a commit that referenced this issue Jan 28, 2021
enhance(parser): Support bracket notation when parsing event names (#1)
@alexprey alexprey added this to the 4.0.0 milestone Feb 13, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request Svelte V2 Issue related to Svelte V2 components Svelte V3 Issue related to Svelte V3 components
Projects
None yet
3 participants