Remove yamlparser dependency

[tyxla]

Seems like we introduced yamlparser as a dependency in #30768. It doesn't seem to be used, though - @sgomes were we planning to use it for anything? If not, it could be removed safely.

Changes proposed in this Pull Request

• Remove yamlparser dependency

Testing instructions

• Checkout this branch.
• Run npm ci and verify Calypso builds well.

[matticbot]

Test live: https://calypso.live/?branch=remove/yamlparser-dependency

[matticbot]

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

[tyxla]

Actually, it seems like this is necessary by useragent for the desktop environment. The interesting fact though is that it's a dev dependency there. Should we alter it to be a dev dependency here, too?

[sgomes]

Hey @tyxla 👋

The issue is that useragent (one of our transitive dependencies) requires yamlparser in some situations (see line 15 in https://unpkg.com/browse/useragent@2.3.0/lib/update.js). Unfortunately, it specifies it as a devDependency, rather than a dependency in its package.json, even though it can be used at runtime.

As such, we needed to explicitly depend on this ourselves.

It's unclear to me why some builds need lib/update.js and some don't, however, so perhaps that's one way of fixing the issue?

[jsnajdr]

I also tried to remove yamlparser when working on #38190, but as @sgomes said, it's a workaround for a bug in useragent/package.json, which specifies its dependencies incorrectly.

When one removes yamlparser, the desktop server build fails. Why doesn't the classic server fail, too? I think it's because the server build externalizes all require() calls that resolve to node_modules. Such require() calls are not resolved by webpack and the required libraries are not bundled. The require() calls are left in the bundled JS code and Node.js resolves them at runtime.

Because the yamlparser is required dynamically, inside a function, you'd need to call that function to trigger a runtime error. And apparently we never call that particular function.

When building the desktop server and bundling all node_modules into the output bundle, the require calls are resolved statically and the build fails.

[tyxla]

Thanks for clarifying, guys.

At this point, I'd call this a false alarm then. It's a shame it's being depended on in such a convoluted and non-transparent way, though.

Closing.

[jsnajdr]

It's a pity we can't add explanatory comments to package.json. 😢 This dependency would deserve one.

[sgomes]

It's a pity we can't add explanatory comments to package.json. 😢 This dependency would deserve one.

We could technically add them, as new keys. Since yamlparser is right at the bottom, we could do something like:

{
  "dependencies": {
    "...": "...",
    "yamlparser": "0.0.2"
  },
  "__comment__": "See https://github.com/Automattic/wp-calypso/pull/39242 for why we need yamlparser"
}

Not pretty, though 😕