update babylon to babel/parser 7.x

[firsttris]

Hey jest-community,

i update babylon to babel/parser 7.x as suggested by @seanpoulter https://github.com/jest-community/jest-editor-support/issues/29

now the typescriptt_parser.js is actually no longer relevant and could possibly be removed. also the typescript dependency in package.json. since the babel_parser.js is now able to parse typescript as well.

i did not delete typescript_parser.js and corresponding test yet. (even if it would no longer be used after this PR)

i add a addtional test - babel_typescript_parser.test.js which executes babel_parser in typescript mode.

i checked that all tests are green, and fixed a few eslint issues.

if you are happy with this PR, i would proceed update the jest parser to new features like test.each.

regards Tristan

[connectdotz]

@firsttris thanks for the PR, before diving into the code, here is a conversation over a year ago that you might find relevant.

IMHO, the most uncertain part of this migration is the plugin list. We used to load all the plugin (yeah, that is a bit crazy), now with an explicit plugin list, I am not sure if we will encounter issues that the user plugin list != our parser's. Will that be an issue for parsing their code? If it does, do we need to consider allow users to pass in their plugin list or even reading their babel config? (a can of worms...) Thoughts?

[firsttris]

reading the users babel config sounds complex to me . big fan of kiss principle.

if the parser supports more features that then user is using everything works well. if the parser does not support features the user is using it will probably not parse the code and generate errors.

if adding all plugins has worked before.. why not continue this way?

i tried to add as many plugins as possible. there was no measurable speed difference when adding all those plugin. (did a tests but exacly the same ms when executing tests)

some plugins:

• decorators
• pipelineOperator did not work

they outputed this errors: The 'decorators' plugin requires a 'decoratorsBeforeExport' option, whose value must be a boolean. If you are migrating from Babylon/Babel 6 or want to use the old decorators proposal, you should use the 'decorators-legacy' plugin instead of 'decorators'. 'pipelineOperator' requires 'proposal' option whose value should be one of: 'minimal', 'smart', 'fsharp'

i actually added them as an array of strings?

also removed the typescript_parser as @stephtr suggested.

[connectdotz]

if adding all plugins has worked before.. why not continue this way?

reasonable starting point.

some plugins ... did not work

see discussion

i actually added them as an array of strings?

should be... but we should probably create some fixtures with some of these features, such as optional-chaining, dynamic import etc, to make sure they can be parsed.

[stephtr]

Will that be an issue for parsing their code? If it does, do we need to consider allow users to pass in their plugin list or even reading their Babel config?

There could be situations where Babel won't work with our configuration, but I'm quite sure that also the Babylon parser (as it is currently used) would have failed for them, it therefore wouldn't be a regression.

[seanpoulter]

Oh!! I am tired and forgot to ask about Yarn. Does Yarn update package-lock.json now too or did you use npm? I'm not bothered by the politics of it, I just want to make sure we're all on board with our build process (which seems to be undocumented).

[firsttris]

are going to do the actual parser change for the test.each in a separate PR, this is just to switch over to the babel-7 world, right?

yes, in those enterprises they told to always keep my PR's small :)

i don't wanted to extend two parsers.

[firsttris]

Oh!! I am tired and forgot to ask about Yarn. Does Yarn update package-lock.json now too or did you use npm? I'm not bothered by the politics of it, I just want to make sure we're all on board with our build process (which seems to be undocumented).

i think i used npm, as far as i know package-lock.json is only created by npm

[connectdotz]

i think i used npm, as far as i know package-lock.json is only created by npm

I wonder if we allow both yarn and npm lock files, will we encounter a situation that developers will end up with different versions of the dependencies based on their pm preference? Or otherwise we will ask developers to update both lock files by running boh yarn and npm? 🤔 Maybe it is best to standardize on the PM, which right now is yarn...

[firsttris]

@rossknudsen just noticed with "allowJs": false in tsconfig its not possible to jump into the javascript files from typescript files with strg + click in vscode. But probably there is a reason for "allowJs": false ...

[rossknudsen]

@firsttris yeah it's because the JS files have Flow typings in them. TS compiler doesn't understand them. Which is why we're using Babel to transpile, as it understands everything.

So the tsconfig is really only being used to typecheck the TS files and we want it to ignore the JS files.

[firsttris]

hey @connectdotz & jest-community, i did my best to implement your suggestions and change requests.

If you have additional change request please let me know. once i find time between my worktime, i will do my best to implement them.

really want to continue extend that parser with test.each support, i see many projects heavely using this and our addons are not supporting this feature.

From the user's perspective, that's a bug. https://github.com/jest-community/vscode-jest/issues/427 https://github.com/firsttris/vscode-jest-runner/issues/55

[connectdotz]

hey @firsttris sorry for the long delay, was really busy this month, I will try to take a look this weekend, thanks for being patient.

[firsttris]

hey @connectdotz to be honest is was not totally clear for me what tasks where missing to complete this PR. the PR is really big, and its seems not really transparent in general on github.

to summarize

the open tasks are:

• we remove the option to pass additional plugins, instead we include all plugins
• update readme accordingly
• update changelog (add author)

do you see any additional changes to be done @connectdotz ? (sorry if i missed something the PR is so big)

[connectdotz]

@firsttris your summary is right, I will take another look when you are done...

[firsttris]

did it

but travis now says

The command "eval yarn --frozen-lockfile " failed 3 times. The command "yarn --frozen-lockfile" failed and exited with 1 during .

what can i do about it?

[rossknudsen]

This is the error:

error semver@7.3.2: The engine "node" is incompatible with this module. Expected version ">=10". Got "8.17.0"

I think you need to change the node version to 10 in the Travis config. I think I had to do that in my PR.

[firsttris]

thx @rossknudsen that worked.

just coveralls is complaining now.

[connectdotz]

hmm... on the 2nd thought, I think this is a good enough start, I can fix the remaining minor issues in another PR so we can free you to move on to the original thing you wanted to fix:

if you are happy with this PR, i would proceed update the jest parser to new features like test.each.

@firsttris if there is anything you want to add I can wait, otherwise I can merge it tonight.

[firsttris]

@connectdotz i'm rdy

[connectdotz]

@firsttris thank you for your hard work 🙏 and sorry it took so long.

[seanpoulter]

Yay! Thank you Tristan! 🥇

[connectdotz]

@firsttris while testing the new typescript babel_parser, noticed a discrepancy vs the old typescript parser. While the old parser does not return the right "name" for the jest.each, it still returns the test block. The new parser completely skipped the block...

If you already started to work on the jest.each then I will just wait, otherwise, I can take a look to at least closing the parity...

[firsttris]

Have not yet started looking in to this topic.

[connectdotz]

ok, I did some quick fix at #47, would love to have you review it.