Chore: Remove esprima #13
Conversation
|
@nzakas What are your thoughts on the "with" tests? |
|
@corbinu Are you referring to the with-scope.js test. Is it failing to parse or failing an assertion? |
|
@soda0289 Yes and the optimization. espree is returning a parsing error as it says with is not allowed in strict mode. Still shows up even if I tell espree strict mode is off. I am just wondering if it is even worth fixing given that I am pretty sure everybody just wants to see "with" go away. |
|
Can you set the sourceType to script. I belive it an option that can be passed into espree |
|
Also I think if ecmaVersion is greater than 5 strict mode is set. Checkout the Readme on espree |
|
The readme lead me to believe all I should have to do is ecmaFeatures.impliedStrict: false |
|
Looks like u cannot pass in options into parse since they are predefined in utils/espree.js. You will probably have to update the espree utility to have it understand different options. |
|
Im pretty sure If sourceType is module strict is set |
|
@soda0289 Got figured out thanks |
|
@JamesHenry should be all set! |
|
Just checking anything I still need to do here? |
|
@soda0289 Can you take a look at this I seem to have somehow blown up the Makefile (it isn't running the tests and isn't printing lint errors) but I didn't edit the Makefile and only removed esprima from the package.json. I currently have left 2 linting errors in the tests to show that it should be erroring and is not. If I log out the command exced in the make file and run it directly I get the correct results |
|
@corbinu I changed the way the tests get run. We now use istanbul to check code coverage. That is run from the node_modules/istanbul directory. Did you run npm install after rebasing? What error do you get when you run |
|
@soda0289 Yes I did a clean npm install. It doesn't error thats the issue it passes silently. Take a look at the latest Travis build https://travis-ci.org/eslint/eslint-scope/jobs/202411080 |
|
@corbinu That is strange. I will have to look into this more tonight. Maybe the makefile variable TEST_SCRIPT is not evaluating or the globing |
|
@soda0289 The tests run fine on master. It seams that the silent failing is caused when I create a directory in tests. In this case the "util" directory. When I created that on master it implodes |
|
Hmmm it must be the globing then. You can change it to test/*.js that will avoid going into the utils directory |
|
@soda0289 That did the trick thanks. @JamesHenry Should be all ready will wait on a review just wanted to make sure there wasn't something anybody saw that needed changing :) |
|
Awesome, thanks everyone! |
|
Oh damn 😦 @corbinu it seems somehow your rebasing ended up putting the wrong commit message in as the final commit for the PR I think the only way to fix it without rebasing master (which I think block anyway) is to revert the PR and resubmit. I have opened a PR to revert this one. |
|
@corbinu please could you resubmit this PR with the correct commit message? |
This builds on #11 and should land after it. It removes esprima and replaces it with espree in all the tests.
It currently fails on two tests that resolve around the use of "with" I am not sure if there is a simple fix or if given that with is not even supported in strict mode if these should simply be removed.