fix: prevent crashing error if more closing tags than opening tags

[silbinarywolf]

I tried to run tests locally and write a test for this case... but had a lot of trouble and I don't really have to time to dig into the build configuration of this repo.

Make sure you check the following:

• [x] you are making a pull request for the develop branch
• [x] you've ran gulp test and it passes the lint 'gulp test' doesn't work on Windows seemingly. I get a lot of errors. template-lint/node_modules/aurelia-pal-nodejs/dist/dom.d.ts(50,35): error TS1005: ',' expected. <- Got 100+ errors like this. Also got CRLF vs LF errors.
• [x] tests don't fail (except tests in 'failing.spec.ts') ** Uncertain... couldn't run them.

Untested test case:

t("avoid crash if too many closing tags", (done) => {
    var config: Config = new Config();
    var linter: AureliaLinter = new AureliaLinter(config);
    var html = `
        <template>
          <div>
            <section>
            </section>
          </div>
          </div>
        </template>`;
    linter.lint(html)
      .then((issues) => {
        // TODO: Jake: Run the test
        expect(issues.length).toBe(1);
        expect(issues[0].message).toBe("Need to see what error prints out and replace this text");
        done();
      });
  });

Cross PR'd here:

• https://github.com/MeirionHughes/aurelia-template-lint/pull/1
  • I wasn't sure where this belonged, so I opened the PR on both.

[silbinarywolf]

The TypeScript errors I mentioned seem to occur in TravisCI as well, however the LF issues are only on my machine https://travis-ci.org/aurelia/template-lint/builds/467259804#L461

[MeirionHughes]

@silbinarywolf regretfully I had to transfer it to aurelia's ownership as a) we switched to Vue in my work and b) my personal time is spent on other projects. I've archived my fork. I don't know who is maintaining it here, sorry.

-I'm not really surprised the testing and stuff is breaking, I was doing a lot of hacks to use functions from the internals of typescript and aurelia.

[silbinarywolf]

Thanks for at least letting me know what was going on :)

[silbinarywolf]

@EisenbergEffect Is there anybody that has any sort of ownership over this module?

[EisenbergEffect]

@silbinarywolf We haven't had anyone take over working regularly on this module since @MeirionHughes had to step back. Are you interested in helping out?

[silbinarywolf]

I don't really have the resources or spare time to commit to becoming a maintainer of this module. However I can see if I can set aside some resources to at least fix the tests in CI and improve the linter, as it'd be in our best interests for the linter to continue working into the future :)

[EisenbergEffect]

That sounds good to me.

[silbinarywolf]

Looking into this now. Will hopefully have fixes in place within the next few days at the most.

[EisenbergEffect]

Thanks for following back up @silbinarywolf ! Looking forward to seeing what you come up with 😄

[silbinarywolf]

After spending a couple of hours on this I've come to the conclusion that touching this up is a far bigger job than I anticipated. The unconventional build system (relative to other Aurelia libraries) and the unhelpful error messages really make it hard to figure out what's broken and whats working fine.

Everytime I fixed a problem, I ended up creating new ones that are hard to reason about and annoying to debug. I'd fixed the typescript and the gulp-typescript library, only to get errors regarding NodeFlags. I fixed those only to get obtuse JS import errors. (I pushed up my broken work in progress here for reference)

I don't think the gains of having a stronger linter is high enough for how much effort is required to fix this.

If I revisit this in the future, I'd probably first scope out a plan and see if you agree with the tools used. Then I'd proceed to do a big PR wherein I use the original code/tests as a guide and I rewrite the whole thing.

In the meantime, are you OK with just merging this fix for now?

[EisenbergEffect]

@silbinarywolf I understand. I've gone ahead and merged this fix.

[silbinarywolf]

Thank you :) If you could also give it a tag on Github + publish to npm, that'd be great.

Just ping @ me when you've done that.

[EisenbergEffect]

@MeirionHughes Does @AureliaEffect have access to publish to that package? We haven't published since you transferred the repo to us. Can you add AureliaEffect on NPM as an owner of that package?

[MeirionHughes]

Hi, I think originally only transferred one of them originally; I've transferred all of them now including the gulp helpers.

[EisenbergEffect]

Thanks @MeirionHughes !