Test failures in latest `woofmark` update due to ES6 inclusion

[jywarren]

trafficstars

In https://github.com/publiclab/PublicLab.Editor/pull/597 we started seeing test failures - @keshav234156 opened https://github.com/publiclab/PublicLab.Editor/pull/602 to revert.

However, the test failures don't trigger a Travis failure, so we weren't seeing them; see this for example:

https://travis-ci.com/github/publiclab/PublicLab.Editor/jobs/379516209

     ReferenceError: Can't find variable: PL in file:///home/travis/build/publiclab/PublicLab.Editor/spec/javascripts/button_spec.js (line 7) (1)
     Expected false to be true. (2)

It looks like potentially a syntax error in our Jasmine tests, under jasmine:publiclabeditor.

The commits were:

Bumps woofmark from 12e2c1d to 39237b7.

• 39237b7 heading test (#34)
• 7a47ab7 Fix .gitpod.yml syntax (missing '-' in tasks) (#28)
• 3a346a6 Bump jest from 26.1.0 to 26.2.2 (#36)
• a5b9afa Bump browserify from 8.1.0 to 16.5.2 (#38)
• a35205a Bump jshint from 2.11.1 to 2.12.0 (#37)
• 8160220 Replacing uglify-js with Terser (#29)
• e6f56ea fix bold texts loosing formatting after converting to markdown and back to wy...
• e9e7bb4 add dependencies for tests and modify jest.config (#17)
• 9bd0750 Install gitpod for auto-built previews on each PR (#16)
• 4f8d765 Merge pull request #13 from NitinBhasneria/latestChanges
• Additional commits viewable in compare view

Let's take a close look and see what could have caused this. Notice that Jest tests did not fail.

[jywarren]

Is this due to Jasmine being unable to read es6, perhaps? @sagarpreet-chadha @Shulammite-Aso @shreyaa-sharmaa @NitinBhasneria @keshav234156 - any thoughts? I notice that we added Terser to woofmark in the meantime?

[keshav234156]

Actually it's difficult to find as we are able to find only when we integrate it with Editor. It's due to an update that we have made in woofmark and it doesn't get noticed when we test them separatly @jywarren Do you know any way we can test this?

[jywarren]

I'm not sure ... the fact that Jasmine is not finding it at all seems to be a big hint. And yet it passes Jest tests.

We haven't really changed the Jasmine setup, i think? https://github.com/publiclab/PublicLab.Editor/tree/main/spec/javascripts (one change 18 days ago)

This to me suggests that it's a difference in the environment somehow. I know Jasmine has trouble with ES6 and we don't have Babel installed to deal with this... ?

Also noting connection to a previous issue with Travis here which ended up being related to Node version: https://github.com/publiclab/PublicLab.Editor/issues/439 - and there, we asked @HarshKhandeparkar and @VladimirMikulic for help. If we get really stuck let's see if they have suggestions, as they are really good at this stuff?

Thanks!

[sagarpreet-chadha]

I agree with @jywarren , it maybe due to es6 usage in upstream library - woofmark. I saw that we are now telling jshint that the code is written in es6 here: https://github.com/jywarren/woofmark/blob/39237b798308e6855acde1a938c18a8e4da62fce/.jshintrc#L2

Maybe this is causing issue?

[sagarpreet-chadha]

I think it is due to use of const keyword which is es6 construct (I only asked asked people to change var to const in some PR's)

[sagarpreet-chadha]

We are using const and even let also here https://github.com/jywarren/woofmark/commit/e6f56ea701b4c74a0e0c1096f056ee8f4fe4186f

@Shulammite-Aso would you like to test if this is the issue?

There are 2 things we can do now in woofmark:

1. Remove let and const everywhere and then try installing this branch in PL.editor by changing package.json (linking woofmark to your branch) and then running Jasmine tests locally.

2. Setup a transpiler like babel which will first convert ES6 to lower ECMA version and then later bundle them into /dist folder. (This we should do anyways)

Thanks :smile:

[jywarren]

Awesome. I guess I feel like option 2 makes sense, but it is a bit of work. @sagarpreet-chadha has actually done this (although i didn't review it 😭 ) in this PR so you can see how it's usually configured!

https://github.com/publiclab/leaflet-environmental-layers/pull/240/files

[Shulammite-Aso]

Hi @sagarpreet-chadha @jywarren is anybody working on this yet? Things must have gotten over my head. I thought i read you were working on this @sagarpreet-chadha ? Just reading through again and saw that wasn't what you said.

I should try setting this up if nobody is already on it, and hit you up @sagarpreet-chadha if i have any question.

[jywarren]

Thank you so much! we had gotten a little behind on this but your help is SO APPRECIATED. This is a big blocker! Looking at your solution now. Thanks!! 🎉