chore: fix css-element-queries import

[manzt]

The css-element-queries package is causing real issues in gosling-widget (and likely and other systems using requirejs).

• Move to latest gosling.js gosling-widget#1

It seems to stem from this annonymous define in ElementQueries.js.

https://github.com/marcj/css-element-queries/blob/4eae4654f4683923153d8dd8f5c0d1bc2067b2a8/src/ElementQueries.js#L10

When importing from css-element-queries, the index.js file is commonjs with two exports:

module.exports = {
    ResizeSensor: require('./src/ResizeSensor'),
    ElementQueries: require('./src/ElementQueries')
};

Commonjs modules cannot be tree-shaken, so all the code for ResizeSensor and ElementQueries end up in resulting bundles even though we don't use ElementQueries at all in the code.

The error in gosling-widget arises as a side-effect ./src/ElementQueries.js inspects the global namespace for define when loaded. We don't see this in the Editor or when loading gosling via a script tag since define is not provided as a global.

I think we can avoid this issue all together with changes suggested in the PR by avoiding the import for ElementQueries all together.

As a side note, we should be thoughtful when adding new NPM dependencies since how they are packaged can have serious implications on where gosling.js can work. I would look for dependencies which are widely used, adhere to modern standards, and frequently updated.

[sehilyi]

Thank you for exploring this issue. This is my fault. I used css-element-queries over other modern packages in the hope that reusing a HiGlass dependency may make things easier, but it wasn't.

I wonder if we can somehow check similar issues at the early stage in this repo (without needing to deploy/test in gosling-widget) whenever we add dependencies in the future.

[manzt]

Seriously no worries! It was just tricky to track down what was going on since the original error in Jupyter was so opaque.

I wonder if we can somehow check similar issues at the early stage in this repo (without needing to deploy/test in gosling-widget) whenever we add dependencies in the future.

Agreed, and unfortunately don't have any solutions off the top of my head. A primary issue is that the feedback loop of making changes to Gosling library and trying the final "build" in different applications is too long (as you've pointed out). It's also important to note that the source code for the Gosling library (src/*) is bundled for the Editor rather than any of the final builds (dist/gosling.es.js). This means we aren't exactly dog-fooding Gosling.js as a package for the editor. To do that, we could need to import gosling.js from dist/*. We also don't have any automated sanity checks for the UMD build of Gosling either (ensuring that dist/gosling.js can be loaded via a script tag).

This was part of the motivation for #697, which is still in progress. The idea there is to have true separation of "packages" for Gosling. With a monorepo, we could actually move gosling-theme and gosling-widget as packages and have a much tighter feedback loop.

[manzt]

In the meantime, some thoughts to hopefully make us less susceptible to issues like this in the future.

• Avoid adding dependencies if possible. npm is an incredibly rough surface with packages using various module systems (CJS, UMD, AMD, ESM) with a wide variety of environment targets (node.js, specific browsers, etc). This makes a lot of work for bundlers since it needs cover all edge cases. Often times older packages will have code execute at runtime (on import) in an attempt to run universally (e.g., inspecting the global for define, exports, process, __webpack__*). We use modern ESM for Gosling's distribution, so we have more confidence that our own source code will be able to be bundled in comparison to others.

• Look at Web APIs for a feature before npm. Remember we are targeting modern browsers (Chrome, Firefox, Safari), and that a native Web APIs which can very often be used instead of some package. Web APIs are stable standards (rarely changed or deprecated) and will not need updating like a npm package (e.g. bumping versions). Using a Web API is basically "free" since there is no worry about how the feature is packaged and the APIs are native to the browsers.

• If you do need a dependency from npm, look carefully at the source code and it's dependencies. Inclusion of a dependency shouldn't be taken lightly and requires several considerations. When was the last time the package was updated? How active is the community? Is the package designed for browsers or does it rely on node builtins (fs, util, Buffer) that will need to polyfill? If you just require a single function from a dependency, it can be easier to copy and paste the source code with a comment into our repo.

EDIT: It's also worth noting that higlass bundles all of it's own dependencies in dist/hglib.js (except react, react-dom, and pixi.js) so our bundler does not have the ability to reuse shared modules at build time. Code will be duplicated between the projects if modules are reused, so it should be preferred to find a more modern/smaller tool if available.

[sehilyi]

Thanks a lot for summarizing these! I will keep those in mind when adding new dependencies. Also, I think it might be valuable to add these instructions to our contributing guidelines.

This was part of the motivation for #697, which is still in progress. The idea there is to have true separation of "packages" for Gosling. With a monorepo, we could actually move gosling-theme and gosling-widget as packages and have a much tighter feedback loop.

This sounds really helpful!