Skip to content
This repository has been archived by the owner on Jan 14, 2019. It is now read-only.

test: add alignment test for espree & change babel sourceType to unambiguous #52

Closed
wants to merge 3 commits into from

Conversation

armano2
Copy link
Contributor

@armano2 armano2 commented Dec 19, 2018

This PR adds alignment test for espree (only for js/jsx files) and changes babel sourceType to unambiguous, additionally i found out that javascript/basic tests are not run in there

espree alignment test

All registered js/jsx tests are going to be processed by espree (eslint parser), while testing i found out that we are missing field in AST:

sourceType unambiguous:

Babel added autodetection of sourceType, with that i was able to remove most of parseWithSourceTypeModule and leave it in only 4 files
there is one more issue with autodetection in babel: babel/babel#9213

@armano2 armano2 changed the title chore: enable valid alignment tests and update tickets links test: add alignment test for espree & change babel sourceType to unambiguous Dec 20, 2018
@JamesHenry
Copy link
Owner

@armano2 I first want to reiterate how grateful I am for your recent contributions, it's been truly awesome!

That gratitude therefore makes me feel very guilty about not being able to accept this PR this time.

Please open issues to discuss major ideas like this in the future, I really hate to render this effort wasted, but I just don't agree with the major shift in direction.

There are a number of reasons why and I wish we'd had a chance to run through them in advance:

  • I don't want to split focus from alignment with babel, as you have already pointed out your changes would require us to disable existing compatibility with babel in favour of more diluted support for other parsers
  • it's hard to know what forms conflicts like those could take in the future, and it just adds to the overhead and challenges around design and decision-making
  • espree is only ever going to be partially relevant, it will not be able to parse TypeScript constructs and this is a TypeScript-focused project
  • it, therefore, adds a lot of complexity without any obvious payoff

I can only thank you again for everything, and hope that we can discuss major ideas like this on issues in the future before spending time on implementation, I'm really sorry that this won't be used.

@JamesHenry JamesHenry closed this Dec 21, 2018
@armano2
Copy link
Contributor Author

armano2 commented Dec 21, 2018

@JamesHenry ok, than i'm going to extract changes about sourceType from babel (it will allow to enable a lot of integration tests)

is it ok?

@armano2
Copy link
Contributor Author

armano2 commented Dec 21, 2018

about #2, i think i explained it incorrectly, babel is not implementing this feature.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants