Update Tern & Acorn

[marcelgerber]

Since Acorn's build process changed (there are no more prebuilt dist files in the repo, but in the npm package), I've switched from having it available as a submodule to just having the files in the file tree. Hope that's fine.
The build is based on my fork https://github.com/MarcelGerber/acorn, where I removed a build step that doesn't quite work with Brackets.

Fixes #11568, and hopefully some others, too.

Commit logs:
Tern + Release notes
Acorn

[marcelgerber]

All tests pass now.

[marcelgerber]

Woah. Looks like this upgrade actually finally fixes the problems we had with Ionic/Cordova apps (cc @MiguelCastillo - would be very kind of you if you could test that). That'd be awesome.
(At least I cannot repro any longer with an example project where I can repro on master)

[ficristo]

Out of curiosity, can I know why they can't be dependencies in package.json?

[marcelgerber]

The two libraries have to be located in the extension's folder, thus they cannot be part of the root package.json.
I don't know for sure why they are not part of a package.json in the thirdparty folder itself, but I guess it's just so you can start "hacking" Brackets in as few steps as possible, where npm install is not needed.

[marcelgerber]

I just now updated this to feature the latest Acorn & Tern versions, where Tern now has ES6 support (#11644).
If you'd rather have the "normal" Tern upgrade first (it's presumably more stable), let me know and I'll factor the ES6 one out into a new PR.

cc @redmunds @nethip for review. Would like to get this in fast so it has a long testing phase on master.

[MiguelCastillo]

@marcelgerber - tern 0.15 is just released a few hours ago. I think that it will be worth doing that version since ES6 has been more thoroughly tested and bugs fixed. That should definitely improve things.

Some things to keep in mind is that there is a new plugin called module, which needs to be enabled in order to es6 to work well. For example, this is what my tern config looks like now.

{
    "libs": [
        "browser",
        "chai",
        "ecma5",
        "ecma6",
        "browser",
        "jquery"
    ],
    "loadEagerly": [],
    "async": true,
    "plugins": {
        "doc_comment": {},
        "requirejs": {},
        "es_modules": {},
        "modules": {}
    }
}

[marcelgerber]

Thanks @MiguelCastillo
Updating Tern was already on my agenda, and I just committed and pushed that upgrade.

I'll try to look at the modules plugin after my 4-day-trip to Berlin, on Sunday/Monday.
Or if you feel like it, feel free to do this yourself :)

[abose]

As this is a core change, Moving to next release as all external integrations happen at the beginning of the cycle.
@marcelgerber Will target this immediately after rel 1.5 is out.

[ingorichter]

@abose what is the timeline for the 1.5 release? I'm just asking, since it's not clear to me, if this means we are holding off PR's for days, weeks or months.

[abose]

1.5 is due within the next 2 weeks and the move to release branch is by next week.
So the holdoff is probably till next week :)

[ingorichter]

That's great @abose! Then let's hold it off until then.

[marcelgerber]

Updated once again to include the latest & greatest.

[busykai]

@marcelgerber, accidentally tried more or less the same and hit the unfortunate change in acorn as well. I think replacing it with a copy is the right thing to do, except these doubts:

1. why do we need all the rest of the files in thirdparty/acorn? perhaps we only need to keep the LICENSE.md, remove the rest except dist/*, and move all the files from dist/ to the root (or leave it there if tern relies on them being in dist).
2. we need to be explicit in the comments which version/commit id of acorn is being committed. since it's not a submodule any longer, it's needed for the bookkeeping.

[marcelgerber]

@busykai Where'd you suggest putting the commit SHA (we already have the version embedded in code)?
Otherwise, we could also simply settle with release versions, so we can rely on the version number by itself, not having to worry about the SHA.

[busykai]

@marcelgerber, I was thinking additionally a version tag/sha in the commit comment. So that you could see with git log the history of the versions.

[marcelgerber]

Ah yes, that makes sense.
But considering I'll rebase these commits away anyhow at some point, we can do that later on.
For now, I'd like to see whether this actually works as expected.

[busykai]

@marcelgerber, understood. What about removing all the acorn junk? We don't need it there (provided we know where to take it from): since it's no longer a submodule, having it in thirdparty/acorn does not make any sense.

I'll take a look at it functionally and run some tests this weekend. @MiguelCastillo also was going to get back to this.

[MiguelCastillo]

@marcelgerber @busykai Ok, I had to read the whole thing again. :)

I'd really prefer using npm packages. It just makes our lives easier because we don't need to worry maintaining yet another repo.

One thing that I think we can do as a separate PR is to move thirdparty deps into the package.json as way to manage dependency upgrades. And we can manage copying these modules from node_modules into thirdparty (or any other place) in our gruntfile.js, which we already do for a few other things. I can look into that separately. But for now, adding tern as a dependency and adding a config in the gruntfile.js to copy the needed files to the right place will be a more straight forward workflow for future upgrades. I see that there are concerns with setting up brackets for hacking. I am not too concerned about that because we will be checking in the dependencies into the thirdparty directory anyways. Upgrades will continue to be a manual process of running npm install and putting out a PR to merge the new stuff into the correct place. No more git submodule init -u ... -whtver please :D

I am going to pull this down right now and take it out for test.

[ficristo]

@MiguelCastillo you should take a look at #12006

[MiguelCastillo]

@ficristo Well thank you very much sir!

[zaggino]

Rebased, squashed & ready to review.

[marcelgerber]

Hm, there's a merge conflict in npm-install.js now. As you definitely know that code better than I do, I'll leave it to you, ok?

[zaggino]

Resolved the conflict @marcelgerber

[marcelgerber]

Another failure: Use the global "use strict" for fix-acorn.js

[zaggino]

🤦 fixed

[ficristo]

If using node_modules or requirejs is too hard maybe we could go with browserify?
Just throwing the idea.

[zaggino]

a) introducing a new tool when you can fix the script with 6 lines of code doesn't appeal to me
b) this should be ported to NodeDomain for improved performance and then you won't have to fix acorn for requirejs

[marcelgerber]

Is this file supposed to have all these packages in there?

[zaggino]

yes - that's how shrinkwrap works, it's generated by npm shrinkwrap command, I'd write more about it but it's easy to google

[marcelgerber]

LGTM, just keen to know if npm-shrinkwrap.json is supposed to contain all these modules.

[zaggino]

LGTM +1

[marcelgerber]

Thank you @zaggino for the work you have done these last few days to bring this up to par and make it mergeable!

[ficristo]

I've run the Extensions \ JavaScript Code Hinting on Windows, OSX and Linux.
While on Windows everything seems smooth, on the other platforms the tests fails.
Could you check?
(There are other tests that do not pass, like JSQuickEdit but I think the problem is the same)

[zaggino]

PR here #12732