js-beautify is using old version causes error [simple fix] [pull request submited] about ecmarkdown

[pravi]

@bakkot @ljharb we don't usually package every node module in debian, only those required by an end user application or development tool. In this case, HedgeDoc is the end user application. Also these tests are run during the package build and when someone runs apt install hedgedoc they don't run the tests. We need to take the source code of every module, build and test them in debian before we ship it to our end users. You can think os us like part of your release team and not a consumer.

[bakkot]

@pravi Thanks for the background.

For context, ecmarkdown is a tool which is used almost exclusively to build the JavaScript language specification (of which @ljharb and I are editors), and proposals for changes to it. reflect-metadata is a nascent (and inactive) such proposal, which happens to also ship a polyfill for the proposed behavior to npm. ecmarkdown is part of its dev-dependencies because it's used to render the specification text in that repository, but it doesn't participate in the production of the runtime code which reflect-metadata publishes, which is presumably what HedgeDoc relies upon (although I don't actually see it in HedgeDoc's dev-dependencies).

That is, ecmarkdown is not in any sense required by the end-user application in question: rather, it is used to build documentation for an experimental project which was at some point used to build HedgeDoc. You can build, test, and run HedgeDoc just fine without running ecmarkdown at all.

All this is to say that I don't think it really makes sense to try to package all of the transitive dev-dependencies of everything you ship. This package has nothing to do with the build or runtime behavior of HedgeDoc, and it's not really a user we intend to support.

[ljharb]

Why would dev dependencies matter to any downstream consumer? Consumers should never be installing this package’s dev deps (or those of any other package).

[vivekkj123]

When we package a npm module to debian, the tests need to be included i.e. the tests need to be executed... In this case the js-beautify is a dependency of your test file...that is why I changed the version of js-beautify on package.json..

[vivekkj123]

There are only 3 ways to solve this issue:-

1. Just change the version in Package.json [ Simple and easy fix] [ Which is done ]

2. Package old version of js-beautify to debian [ Not recommended by debian ]

3. Avoid the usage of js-beautify in this npm module ( in test part) [ Needs hardwork ]

[bakkot]

It is not clear to me why debian is attempting to include this package at all, but in any case I've fixed up the PR and merged it.

[vivekkj123]

We are packing HedgeDoc [1], a useful platform which can be hosted and can be used for writing and sharing markdown notes to Debian...
In this packaging work, we need to pack all dependants that required for this package[2]... Reflect-metadata is a dependency for HedgeDoc... ecmarkdown is a devdependency used in gulp test of this package... This is why we are packing ecmarkdown to Debian...

[1] https://github.com/hedgedoc/hedgedoc
[2] https://wiki.debian.org/Javascript/Nodejs/Tasks/HedgeDoc

Anyway Thank You for merging the pull request... Have a nice day... 😊

[vivekkj123]

Also please tag this as new version in this repo...

[pravi]

We are mostly interested in running tests during build time, this is to help us find issues when we update dependencies. For general purpose tools we are also interested to build documentation. But if ecmarkdown is strictly used only for documentation, we can skip that part. @vivekkj123 skip building the documentation step.

[vivekkj123]

@pravi Ok 😀