chore(core): update build #977

etowahadams · 2023-09-28T22:36:22Z

Fix #
Toward #

Change List

Removes /embed entrypoint
Removes umd build
Adds "exports" field to package.json
Adds knip
Removes unused dependencies
Removes unused files
Removes some unused methods
Make all CI/CD run on Node 20
Updates gosling.js HTML template to use module imports
Upgrade vite and vitest
replace @vitejs/plugin-react-refresh with @vitejs/plugin-react

Checklist

Ensure the PR works with all demos on the online editor
Unit tests added or updated
Examples added or updated
Documentation updated (e.g., added API functions)
Screenshots for visual changes (e.g., new encoding support or UI change on Editor)

etowahadams · 2023-09-28T23:21:00Z

The main thing remaining is to change getHtmlTemplate so that it uses esm.sh or something similar instead of the current unpkg. Will do that tomorrow.

Also also I'm wondering if should get rid of knip since it only supports Node 16+. Or try downgrading it assuming it used to support Node 14. I cut most of the dependencies knip marked as unused. There's also a bunch of exports (export const ...) that don't get used outside of the file they're defined in. I might deleting the export from those later.

etowahadams · 2023-09-29T21:27:16Z

I've been experimenting with using esm.sh. Because HiGlass is UMD and not ESM, esm.sh can't get find any of the exports from HiGlass that Gosling uses.

Uncaught SyntaxError: The requested module '/v132/[email protected]/X-ZC9waXhpLmpzQDYuNS4xMCxyZWFjdC1kb21AMTcuMC4yLHJlYWN0QDE3LjAuMg/es2022/higlass.mjs' does not provide an export named 'HiGlassComponent' (at gosling.mjs:3:2125)

@manzt Do you think it would be worth updating HiGlass to give it a ESM build? I see that what happens now is that it creates a higlass.umd.js which gets transpiled using babel (with plugin transform classes).

The only alternative I can think of is to use importmaps. Its less clean but would probably get the job done.

Here's the HTML I've been playing with:

<!DOCTYPE html>
<html>
  <head>
    <link rel="stylesheet" href="https://esm.sh/[email protected]/dist/hglib.css">
  </head>
  <body>
    <div id="gosling-container"></div>
  </body>
  <script type="module">
    // you can import submodules from esm.sh
    import { embed } from "https://esm.sh/[email protected]?deps=react@17,react-dom@17,pixi.js@6,[email protected]";
    embed(document.getElementById('gosling-container'), {
  "title": "Basic Marks: bar",
  "subtitle": "Tutorial Examples",
  "tracks": [
    {
      "layout": "linear",
      "width": 800,
      "height": 180,
      "data": {
        "url": "https://resgen.io/api/v1/tileset_info/?d=UvVPeLHuRDiYA3qwFlm7xQ",
        "type": "multivec",
        "row": "sample",
        "column": "position",
        "value": "peak",
        "categories": ["sample 1"],
        "binSize": 5
      },
      "mark": "bar",
      "x": {"field": "start", "type": "genomic", "axis": "bottom"},
      "xe": {"field": "end", "type": "genomic"},
      "y": {"field": "peak", "type": "quantitative", "axis": "right"},
      "size": {"value": 5}
    }
  ]
})
    </script>
</html>

manzt · 2023-09-29T21:53:58Z

Excited to see progress on this! Will try to take closer look this weekend. But just wanted to chime in.

Also also I'm wondering if should get rid of knip since it only supports Node 16+.

If gosling really just runs in the browser, I don't really see a motivation to support legacy versions of Node.js for development purposes. Node 14 and 16 are both EOL, meaning they are no longer being developed or will receive security fixes https://endoflife.date/nodejs). I'd look into using Node 20 and maybe consider just testing in that environment. Could reduce CI times.

@manzt Do you think it would be worth updating HiGlass to give it a ESM build? I see that what happens now is that it creates a higlass.umd.js which gets transpiled using babel (with plugin transform classes).

Yeah, I think that would be nice downstream. HiGlass will need to keep UMD for some time, due to its plugin system and other projects that rely on using it through script tags (many more higlass developers).

However, one thing I've been very interested in there is breaking up higlass into smaller packages that are pure ESM. We would then have @higlass/* which something like gosling could depend on, and then just have a top level higlass that imports these things and publishes to npm.

etowahadams · 2023-09-30T02:14:13Z

I'd look into using Node 20 and maybe consider just testing in that environment. Could reduce CI times.

Good idea, I'll try it out!

However, one thing I've been very interested in there is breaking up higlass into smaller packages that are pure ESM

Agreed and I would expect this to be relevant to our previous discussion too.

etowahadams · 2023-10-04T22:56:14Z

edit: using the?bundle option with esm.sh works!

esm.sh

I attempted with importmaps and the esm.sh CDN but without success.

It seems that esm.sh doesn't have the pixi v6.x so it keeps using pixi v5.3.12. I noticed this because the error relates to not being able to import this es6-promise-polyfill package. Not sure why it can't import this package.

/* esm.sh - esbuild bundle(@pixi/[email protected]) es2022 production */
import {Polyfill as f} from "/v132/[email protected]/es2022/es6-promise-polyfill.mjs";

Here is the html:

<!DOCTYPE html>
<html>
  <head>
    <link rel="stylesheet" href="https://unpkg.com/[email protected]/dist/hglib.css">

    <script type="importmap">
    {
    "imports": {
        "react": "https://esm.sh/react@17",
        "react-dom": "https://esm.sh/react-dom@17",
        "pixi.js": "https://esm.sh/pixi.js@6",
        "higlass": "https://esm.sh/[email protected]",
        "gosling.js": "https://esm.sh/[email protected]"
    }
    }
    </script>
<body>
    <div id="gosling-container"></div>
  
    <script type="module">
      import { embed } from 'gosling.js';
    embed(document.getElementById('gosling-container'), {
  .... same spec as above }
    </script>
</body>
</html>

skypack.dev

I also tried using another CDN called skypack.dev, without success, although with a different error.

Uncaught SyntaxError: The requested module 'gosling.js' does not provide an export named 'embed'

This error is a bit surprising because dist/gosling.es.js does have an export called embed

export { GoslingComponent, GoslingSchema, GoslingTemplates, theme_schema as ThemeSchema, compile, embed, init, name, validateGoslingSpec, version };

Next steps

I'm wondering what the best way to go about troubleshooting this. Perhaps there is something obvious I'm missing, or perhaps there's some deeper understanding of how these CDNs work that I'm missing. My hunch is that there's something wrong with the higlass or gosling build that is causing these difficulties? More to investigate.

manzt · 2023-10-06T18:03:27Z

editor/html-template.ts

+        "react": "https://esm.sh/react@${reactVersion}?bundle",
+        "react-dom": "https://esm.sh/react-dom@${reactVersion}?bundle",
+        "pixi.js": "https://esm.sh/pixi.js@${pixiVersion}?bundle",
+        "higlass": "https://esm.sh/higlass@${higlassVersion}?bundle",
+        "gosling.js": "https://esm.sh/gosling.js@${goslingVersion}?bundle"


With ?bundle I'm actually not sure gosling.js has any imports to resolve. I would be curious about just using:

import { embed } from 'https://esm.sh/gosling.js?bundle';

below. The main issue I issue I see with this is that I'm not sure how to control the version of HiGlass so that it aligns with the css.

esm.sh says

In bundle mode, all dependencies are bundled into a single JS file except the peer dependencies.

So actually I don't need to import Higlass! Just react, react-dom, and pixi.js

Actually you're right that does work! I'm surprised though. I'll have to read more about this bundle option.

import { embed } from "https://esm.sh/[email protected]?bundle" embed(document.getElementById('gosling-container'), {...}

etowahadams · 2023-10-06T19:26:51Z

Node 20 has the Web Crypto API built in (globalThis.crypto). But Node 18 doesn't.

Because of this, using changing crypto with vi.stubGlobal or globalThis or window works in Node 18 but not Node 20 (else you get a TypeError: Cannot set property crypto of #<Object> which has only a getter).

What are our options? I'm pretty sure that @manzt 's PR #976 would fix this since it includes a fallback for crypto.randomUUID. So perhaps we update the test Node versions after that PR? Or just only test with Node 20 which is probably fine since Gosling is mainly to be run in a browser?

manzt · 2023-10-06T19:36:17Z

Or just only test with Node 20 which is probably fine since Gosling is mainly to be run in a browser?

Ya, I'm in favor of just using 20. Especially since it better reflects the environment Gosling actually runs in. We just need to communicate the change/requirement to the gosling team. We could also look into adding an .nvmrc, so users of nvm and fnm automatically get the right version of node when the cd into the gosling.js.

etowahadams · 2023-10-06T21:56:22Z

That makes sense to me. I'll add a note about testing using Node 20 to the contributing doc and will add a .nvmrc. We can make an announcement to the Gosling channel when this PR is finalized.
cc: @sehilyi

etowahadams · 2023-10-12T22:39:27Z

Removed mention of nvm based on comments today

manzt · 2023-10-14T22:25:35Z

editor/html-template.ts

-        gosling.embed(document.getElementById('gosling-container'), ${spec})
+    <div id="gosling-container"></div>
+    <script type="module">
+        import { embed } from 'https://esm.sh/gosling.js@${goslingVersion}?bundle';


The only question I have here is whether the version of higlass and gosling.js will stay in sync over time. Unfortunately the names used in the styles are specific to the higlass version, so I don't know if there could be a case where higlass makes a patch (e.g., v1.11.8) but we have styles from 1.11.7 in our template and the two end up out of sync. I wonder if it's possible to do something like:

import { embed } from 'https://esm.sh/gosling.js@${goslingVersion}?bundle&deps=higlass@${higlassVersion}';

but I'm not sure if the deps is handled when bundle is used.

Is it possible to use different react and react-dom versions as well that user wishes to use?

Same question. Ideally you could bundle and override dependency versions, but I'm not sure for ESM.sh if that is current possible.

Just tested. Unfortunately, it seems to ignore the version of the dependencies. I tried using a lower version of HiGlass and I still got HiGlass v1.12.4: http://higlass.io/ in the console. What I'll try next is to use the pre-bundled peer dependencies in an import map.

import { embed } from 'https://esm.sh/[email protected]?bundle&deps=higlass@$1.11.7';

I didn't have any luck using unbundled gosling.js with bundled peer dependencies either using an importmap.

The problem seems to be that HiGlass has "pixi.js": "^5.0.3" as a peer dependency. One of the pixi subpackages called @pixi/polyfill relies on a package called es6-promise-polyfill which doesn't have the export that @pixi/polyfill expects. Thus I get a The requested module '/v133/[email protected]/es2022/es6-promise-polyfill.mjs' does not provide an export named 'Polyfill' error when trying to import HiGlass.

Looking at that version of es6-promise-pollyfill it does look like there is export called Polyfill but I don't think it gets converted in a way that is accessible in the ESM version that esm.sh generates.

How hard it would it be to upgrade the pixi version that HiGlass uses to v6?

Added a PR to HiGlass to upgrade Pixi #977

manzt

Awesome. Thanks for all your work on this Etowah, looking great. Just one thought about esm.sh, otherwise LGTM!

embed/index.ts

knip.config.json

manzt · 2023-10-14T22:30:28Z

package.json

    "module": "dist/gosling.es.js",
    "types": "dist/src/index.d.ts",
    "files": [
        "dist"
    ],
+    "type": "module",


Yup! Making things work nice (like Vite) might require upgrading those deps.

scripts/setup-vitest.js

src/data-fetchers/gff/gff-worker.ts

vite.config.mjs

manzt · 2023-10-14T22:37:49Z

Ah, one thing I forgot. We have branch protection rules that require "Test (14.x)" and "Test (16.x)" to pass to merge into master. That's why "Some checks haven't completed yet". You removed these workflows in this PR, so that's why it's still waiting for them to complete. We should be ok to remove these rules in Settings (since they will never fulfill) and maybe add one for the node test workflow.

package.json

sehilyi · 2023-10-16T18:27:08Z

src/missing-types.d.ts

-    // export * as d3Queue from 'd3-queue';
-    // export * as d3Request from 'd3-request';


Is this intended? Could be removed if not needed.

The exports ford3Queue and d3Request were commented out so I just removed them. Perhaps they're unused in the parts of HiGlass that Gosling uses?

src/tracks/gosling-track/gosling-track-model.ts

sehilyi · 2023-10-16T18:39:14Z

@manzt @etowahadams Removed the "Test (14.x)" and "Test (16.x)" branch protection rules. I will add one for 20.x after this is merged.

manzt · 2023-10-24T01:17:33Z

Released HiGlass v1.13

etowahadams · 2023-10-24T01:41:05Z

Amazing thanks @manzt! Will try it out tomorrow.

etowahadams · 2023-10-24T19:12:23Z

The polyfill error is resolved! Now encountering a new error due to HiGlass not being ESM.

// [OLD] This still uses [email protected] and we get the polyfill import error
import { embed } from 'https://esm.sh/[email protected]';

// [NEW] This uses [email protected]
// No more polyfill error (yay!). However, we get a new error
// higlass.mjs does not provide an export named 'HiGlassComponent
import { embed } from 'https://esm.sh/[email protected][email protected]'

Why is this failing? higlass.js is a UMD module. Esm.sh converts this into a ES module which has a single export:

// https://esm.sh/v133/[email protected]/es2022/higlass.mjs 
export {oNe as default};

So in gosling.mjs, this fails:

import { HiGlassComponent as ps } from "/v133/[email protected]/es2022/higlass.mjs";
// ERROR: higlass.mjs does not provide an export named 'HiGlassComponent

Potential solutions

Add feature to esm.sh

One solution which would require adding a feature to esm.sh. Currently esm.sh allows you to specify named exports from CJS file:

import { HiGlassComponent } from "https://esm.sh/[email protected]?cjs-exports=HiGlassComponent";

This gets turned into the following in higlass.mjs, allowing HiGlassComponent to be imported.

export * from "/v133/[email protected]/X-dHMvSGlHbGFzc0NvbXBvbmVudA/es2022/higlass.mjs";
import __cjs_exports$ from "/v133/[email protected]/X-dHMvSGlHbGFzc0NvbXBvbmVudA/es2022/higlass.mjs";
export const { HiGlassComponent } = __cjs_exports$;

However, you can’t combine this feature with the ?deps option

// This doesn't work :(
import { embed } from 'https://esm.sh/[email protected][email protected]?cjs-exports=HiGlassComponent'

So we would have to submit a PR to esm.sh to add this feature.

ESM build for Higlass

Once again we return to this. I’m ready to make this a priority if others are onboard with me doing so.

Add back the UMD build

We could also just bring back the UMD build as a short term fix to merge this PR. This is tempting.

Thoughts? @manzt @sehilyi

etowahadams · 2023-10-26T19:22:19Z

Just waiting on higlass/higlass#1171 and we should be good to go!

etowahadams · 2023-10-26T21:08:56Z

This should be everything. Thanks for all the work on HiGlass Trevor!

etowahadams added 8 commits September 28, 2023 17:15

feat: remove umd and embed builds

dd65e2f

feat: knip config

26ef52f

refactor: remove unused packages

e8b9a8f

docs: add comments

857889a

refactor: remove unused exports, files

fbc4b01

fix: reconfigure knip

923ba98

fix: remove comments

dc9fbe3

fix: add other workers to manualInlineWorkers

f327ae5

etowahadams changed the title ~~chore: update build~~ chore(core): update build Sep 28, 2023

etowahadams added 5 commits October 5, 2023 18:10

feat: html template using importmaps

2a14b63

chore: upgrade node version in CI

289aff2

fix: bed parser import

8621b94

fix: remove unused crypto mock

77d7753

fix: mock crypto using stubGlobal

8ec6610

manzt reviewed Oct 6, 2023

View reviewed changes

etowahadams added 7 commits October 6, 2023 17:58

fix: only run tests in node 20

1f6de35

feat: nvmrc

5586b72

chore: update html-template

ed8caa7

docs: update contributing guide with node version

394de59

chore: remove crypto patch

2f0334c

chore: remove main from package.json

a877e67

chore: add main and type of package.json

1a926ee

manzt reviewed Oct 14, 2023

View reviewed changes

manzt approved these changes Oct 14, 2023

View reviewed changes

etowahadams added 3 commits October 16, 2023 12:37

chore: upgrade vite, vitest, and plugin-react

17fd4dc

chore: change from ReactDOM.render to createRoot

ca8eea2

fix(tracks): add type to import

2894aa2

sehilyi reviewed Oct 16, 2023

View reviewed changes

package.json Show resolved Hide resolved

sehilyi reviewed Oct 16, 2023

View reviewed changes

package.json Show resolved Hide resolved

sehilyi reviewed Oct 16, 2023

View reviewed changes

src/tracks/gosling-track/gosling-track-model.ts Show resolved Hide resolved

sehilyi approved these changes Oct 16, 2023

View reviewed changes

etowahadams added 3 commits October 16, 2023 18:16

fix(tests): canvas mocking

1e4a694

fix: make coverage work by adding vitest/coverage

f1046ac

fix: remove crypto global

fbac45e

etowahadams mentioned this pull request Oct 16, 2023

embed function handle usage for Observable #981

Open

chore: rename from vite.config.mjs to .js

b88c3ef

sehilyi mentioned this pull request Oct 20, 2023

chore: upgrade pixi higlass/higlass#1163

Merged

7 tasks

etowahadams added 2 commits October 26, 2023 15:13

chore: upgrade higlass version

ed1e6b7

feat: importmap html template

c862ce0

chore: update higlass

562e46d

etowahadams merged commit 5457f16 into master Oct 26, 2023
5 checks passed

etowahadams deleted the etowahadams/clean-build branch October 26, 2023 21:10

etowahadams mentioned this pull request Nov 17, 2023

Bam data fetcher issues in development build #991

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(core): update build #977

chore(core): update build #977

etowahadams commented Sep 28, 2023 •

edited

Loading

etowahadams commented Sep 28, 2023

etowahadams commented Sep 29, 2023

manzt commented Sep 29, 2023 •

edited

Loading

etowahadams commented Sep 30, 2023

etowahadams commented Oct 4, 2023 •

edited

Loading

manzt Oct 6, 2023

etowahadams Oct 6, 2023

etowahadams Oct 6, 2023

etowahadams commented Oct 6, 2023

manzt commented Oct 6, 2023

etowahadams commented Oct 6, 2023

etowahadams commented Oct 12, 2023

manzt Oct 14, 2023 •

edited

Loading

sehilyi Oct 16, 2023

manzt Oct 16, 2023

etowahadams Oct 16, 2023

etowahadams Oct 17, 2023

etowahadams Oct 19, 2023

manzt left a comment

manzt Oct 14, 2023

manzt commented Oct 14, 2023 •

edited

Loading

sehilyi Oct 16, 2023

etowahadams Oct 16, 2023

sehilyi commented Oct 16, 2023

manzt commented Oct 24, 2023 •

edited

Loading

etowahadams commented Oct 24, 2023

etowahadams commented Oct 24, 2023

etowahadams commented Oct 26, 2023

etowahadams commented Oct 26, 2023

		// export * as d3Queue from 'd3-queue';
		// export * as d3Request from 'd3-request';

chore(core): update build #977

chore(core): update build #977

Conversation

etowahadams commented Sep 28, 2023 • edited Loading

Change List

Checklist

etowahadams commented Sep 28, 2023

etowahadams commented Sep 29, 2023

manzt commented Sep 29, 2023 • edited Loading

etowahadams commented Sep 30, 2023

etowahadams commented Oct 4, 2023 • edited Loading

esm.sh

skypack.dev

Next steps

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

etowahadams commented Oct 6, 2023

manzt commented Oct 6, 2023

etowahadams commented Oct 6, 2023

etowahadams commented Oct 12, 2023

manzt Oct 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manzt left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

manzt commented Oct 14, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sehilyi commented Oct 16, 2023

manzt commented Oct 24, 2023 • edited Loading

etowahadams commented Oct 24, 2023

etowahadams commented Oct 24, 2023

Potential solutions

Add feature to esm.sh

ESM build for Higlass

Add back the UMD build

etowahadams commented Oct 26, 2023

etowahadams commented Oct 26, 2023

etowahadams commented Sep 28, 2023 •

edited

Loading

manzt commented Sep 29, 2023 •

edited

Loading

etowahadams commented Oct 4, 2023 •

edited

Loading

manzt Oct 14, 2023 •

edited

Loading

manzt commented Oct 14, 2023 •

edited

Loading

manzt commented Oct 24, 2023 •

edited

Loading