how to include third party javascript libraries?

[fippo]

In this chrome CL I tried adding a third-party library for dealing with SDP in webrtc tests. I put a trimmed down version of the code, a README specifying where to obtain it and a LICENSE file into a directory under the one with the tests using it. @foolip was summoned and asked me to raise the issue here.

How should javascript libraries that are third-party be treated? "Third party" in the sense "they existed before, they are independent from wpt and someone else maintains them"

I assume fetching them from the network is off the table since there should be no runtime dependency on the network.

Should they put in a central place so they can be ignored by linters? If they are only used by a particular spec (nothing else on the web uses SDP thankfully) is a place like <spec>/resources/ better.
Should a package manager (npm, yarn, ...) be used? With or without lockfiles etc.
What is the process for updating them (e.g. npm audit)? Should local modifications be avoided?
Are there licensing concerns that need to be taken care of?

Lots of questions...
in this particular case i'm happy to use the same pattern that is used for webidl2.js

[jgraham]

My point of view:

• Use <spec>/third_party for third party code
• We can write a rule to exclude all third_party directories from linting
• The best way to add them is typically using git subtree, in case local modifications are required (but local modifications are discouraged in general I'd say). Note that merging subtrees required a direct push to preserve the tree shape.
• I don't think we want to use a package manager here if possible. The case where it might not be is where we can't include source directly but need to include the output of a build step.
• Updating is hard; when we vendor stuff we lose the support of most tools that can check for outdated versions (I think).
• License needs to be compatible with the overall BSD license. So things like GPL dependencies are problematic (by which I think I mean "not possible to include")

[Hexcles]

The best way to add them is typically using git subtree, in case local modifications are required (but local modifications are discouraged in general I'd say). Note that merging subtrees required a direct push to preserve the tree shape.
I don't think we want to use a package manager here if possible. The case where it might not be is where we can't include source directly but need to include the output of a build step.
Updating is hard; when we vendor stuff we lose the support of most tools that can check for outdated versions (I think).

Vendoring third-party code and using a package manager are not mutually exclusive, if we choose not to use git subtree.

For npm packages, I actually think we should use a package manager, and check in the fetched source code. This solves the problem both for vendors (use the checked in source code directly) and authors (easy to upgrade using npm up), at the cost of losing the detailed version history (which doesn't really matter as long as we don't make local modifications). We probably want to use yarn for its flat mode.

[fippo]

I took a stab at the "use yarn" approach. Branch here

The package.json files can the pretty small thankfully, only listing dependencies. npm and yarn complain about a missing license field but that is just a warning.
Checking in node_modules required git add -f since node_modules is in the root .gitignore. -f solves the problem however.

One thing I didn't like was checking in the full package including documentation, tests etc. I think having a .gitignore to just 🍒-pick what is needed may reduce the size considerably. But when i looked at least in the case of the sdp package the tests were already not included in the npm package.

[fippo]

made a PR from the branch

[snuggs]

@fippo this can turn into a ton of technical debt very fast as i'm sure you already know. Also not sure what happens from breaking typical node conventions of traversing node_modules location. Then I realized this isn't a node project per se and the minimum we need is sdp.js. Luckily this package is fairly tight. So seems like an easy band aid. Our future selves may not be so lucky with other third_party so great to discuss. :-).

My vote/opinion is for placing sdp.js (wherever is agreed upon), reference it where needed, and call it a day. However this strategy still doesn't avoid needing to check externally for updates. It's already commonplace (i.e. Tree Shaking) to strip what we don't need (down to the method). If we need a license can throw it at the head. Perhaps i'm thinking too lazy but never been told that's a bad thing 😎

FWIW just my 2 satoshis. 👍 👍 on the sdp js lib though. Personally it's been helpful numerous times in the past. 🙏

[fippo]

ping - if you folks could take a look at the PR

@alvestrand just proposed another creative test in which this library would be super helpful :-)

[stephenmcgruer]

Hi all, sorry for the delay. I discussed this with @jgraham and @Hexcles today, and we're happy for the proposal from @snuggs to go ahead. That is:

i. Add a new directory, webrtc/third_party/sdp
ii. Vendor (copy) sdp.js and its associated LICENSE file into webrtc/third_party/sdp
iii. Add lint rules to ensure that third_party directories are excluded from linting presubmits.

This is us (deliberately) deferring on an overall policy for third-party javascript in WPT, but hopefully unblocking your current efforts.

I am happy to work on (iii), and look to yourselves to arrange (i) and (ii), if this meets your needs. Thanks!