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

Allow default export for any declaration #160

Closed
wants to merge 2 commits into from

Conversation

resolritter
Copy link
Contributor

This change would cover the syntax described in tree-sitter/tree-sitter-typescript#125 (comment) without extra grammar additions on tree-sitter-typescript.

@mjambon
Copy link
Contributor

mjambon commented Mar 18, 2021

Hey @dcreager @maxbrunsfeld, could someone merge this please? We're eager to try this out and we can't do so easily because it makes us set up experimental forks for both tree-sitter-javascript and tree-sitter-typescript.

@@ -79,6 +80,9 @@ module.exports = grammar({
[$.assignment_expression, $.object_assignment_pattern],
[$.labeled_statement, $._property_name],
[$.computed_property_name, $.array],
[$.export_clause, $.object, $.object_pattern],
[$._import_export_specifier, $.object, $.object_pattern],
[$.export_statement, $._property_name],
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you report what error you get if you remove these conflicts? I'm assuming that, by allowing export <expression>, we're creating ambiguities between "export clauses (e.g. export {foo as bar, baz}) and exported object literals (e.g. export {foo: bar}).

I'm wondering if the latter case is really allowed, or if we should keep default as required when exporting an expression.

Copy link
Contributor Author

@resolritter resolritter Mar 19, 2021

Choose a reason for hiding this comment

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

The way I comprehend it, this syntax is for exporting identifiers, not objects. It's sugar for export const c = c so you don't have to repeat the c twice. Fairly common to see in the wild.

From what I've tested in the links above, the "object version" of export { foo: bar } is not valid.


The error I get by removing the conflicts is:

Unresolved conflict for symbol sequence:

  'export'  '{'  ','  •  '}'  …

Possible interpretations:

  1:  'export'  '{'  (object_pattern_repeat1  ',')  •  '}'  …
  2:  'export'  '{'  (object_repeat1  ',')  •  '}'  …
  3:  'export'  (export_clause  '{'  ','  •  '}')

Copy link
Contributor

Choose a reason for hiding this comment

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

I pushed the suggested fix, consisting in allowing expressions only after export default, not directly after export.

I made a new PR for this: #168

mjambon added a commit that referenced this pull request Apr 20, 2021
Allow default export for any declaration (extension of #160)
@mjambon
Copy link
Contributor

mjambon commented Apr 20, 2021

Closing this PR since it was extended by #168 which is now merged.

@mjambon mjambon closed this Apr 20, 2021
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