Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

chore(core): update build #977

Merged
merged 36 commits into from
Oct 26, 2023
Merged
Show file tree
Hide file tree
Changes from 26 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
dd65e2f
feat: remove umd and embed builds
etowahadams Sep 28, 2023
26ef52f
feat: knip config
etowahadams Sep 28, 2023
e8b9a8f
refactor: remove unused packages
etowahadams Sep 28, 2023
857889a
docs: add comments
etowahadams Sep 28, 2023
fbc4b01
refactor: remove unused exports, files
etowahadams Sep 28, 2023
923ba98
fix: reconfigure knip
etowahadams Sep 28, 2023
dc9fbe3
fix: remove comments
etowahadams Sep 28, 2023
f327ae5
fix: add other workers to manualInlineWorkers
etowahadams Sep 28, 2023
2a14b63
feat: html template using importmaps
etowahadams Oct 5, 2023
289aff2
chore: upgrade node version in CI
etowahadams Oct 5, 2023
8621b94
fix: bed parser import
etowahadams Oct 5, 2023
77d7753
fix: remove unused crypto mock
etowahadams Oct 6, 2023
8ec6610
fix: mock crypto using stubGlobal
etowahadams Oct 6, 2023
1f6de35
fix: only run tests in node 20
etowahadams Oct 6, 2023
5586b72
feat: nvmrc
etowahadams Oct 6, 2023
ed8caa7
chore: update html-template
etowahadams Oct 6, 2023
394de59
docs: update contributing guide with node version
etowahadams Oct 6, 2023
2f0334c
chore: remove crypto patch
etowahadams Oct 6, 2023
a877e67
chore: remove main from package.json
etowahadams Oct 6, 2023
1a926ee
chore: add main and type of package.json
etowahadams Oct 6, 2023
c79ebbe
chore: get rid of built-in __dirname
etowahadams Oct 7, 2023
e981887
chore: files using module exports make cjs
etowahadams Oct 7, 2023
4cb8c7b
fix: vitest thinks config is cjs, change to mjs
etowahadams Oct 7, 2023
8223c30
fix: target to es2022
etowahadams Oct 7, 2023
415d8ab
chore: update target to esnext
etowahadams Oct 7, 2023
6997405
docs: remove mention of nvm
etowahadams Oct 12, 2023
17fd4dc
chore: upgrade vite, vitest, and plugin-react
etowahadams Oct 16, 2023
ca8eea2
chore: change from ReactDOM.render to createRoot
etowahadams Oct 16, 2023
2894aa2
fix(tracks): add type to import
etowahadams Oct 16, 2023
1e4a694
fix(tests): canvas mocking
etowahadams Oct 16, 2023
f1046ac
fix: make coverage work by adding vitest/coverage
etowahadams Oct 16, 2023
fbac45e
fix: remove crypto global
etowahadams Oct 16, 2023
b88c3ef
chore: rename from vite.config.mjs to .js
etowahadams Oct 16, 2023
ed1e6b7
chore: upgrade higlass version
etowahadams Oct 26, 2023
c862ce0
feat: importmap html template
etowahadams Oct 26, 2023
562e46d
chore: update higlass
etowahadams Oct 26, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
File renamed without changes.
6 changes: 3 additions & 3 deletions .github/workflows/ci.yml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ jobs:
- name: Use Node.js
uses: actions/setup-node@v3
with:
node-version: 16
node-version: 20
- run: yarn install
- run: yarn build-editor
env:
Expand All @@ -30,7 +30,7 @@ jobs:
runs-on: ubuntu-latest
strategy:
matrix:
node-version: [14.x, 16.x]
node-version: [20.x]
steps:
- uses: actions/checkout@v3
- name: Use Node.js ${{ matrix.node-version }}
Expand All @@ -53,7 +53,7 @@ jobs:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 16
node-version: 20
- run: yarn install
- name: ESLint
run: yarn eslint src/ editor/
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/deploy-editor.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ jobs:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 16
node-version: 20
- run: yarn install
- run: yarn build-editor
env:
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ jobs:
- uses: actions/checkout@v3
- uses: actions/setup-node@v3
with:
node-version: 16
node-version: 20
registry-url: https://registry.npmjs.org/
- run: yarn
- run: yarn build
Expand All @@ -30,7 +30,7 @@ jobs:
fetch-depth: 0
- uses: actions/setup-node@v3
with:
node-version: 16
node-version: 20
- run: npx changelogithub
env:
GITHUB_TOKEN: ${{secrets.GITHUB_TOKEN}}
1 change: 1 addition & 0 deletions .nvmrc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
20
12 changes: 9 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ While contribution guidelines are loosely defined, we recommend to read the foll

We use Yarn (and not NPM) to manage dependencies in stable and consistent ways.

After installing [Yarn](https://yarnpkg.com/getting-started/install), clone this repository and run the following commands to install all dependencies and run the Gosling.js editor locally:
After installing [Yarn](https://yarnpkg.com/getting-started/install), clone this repository and run the following commands to install all dependencies and run the Gosling.js editor locally. We recommend you use Node 20.

```sh
yarn
yarn start
git clone https://github.com/gosling-lang/gosling.js.git # Clone the repository to your current directory
cd gosling.js # Navigate to gosling repository
yarn # Install dependencies
yarn start # Start a local server running the Gosling online editor
```

Then, you can open http://localhost:3000/ in a web browser to test the online editor.
Expand Down Expand Up @@ -49,6 +51,10 @@ To learn more about the commitlint, please visit [conventional-changelog/commitl
## Opening Pull Requests
We use the [commitlint](#commitlint) for the title of PR. So, if the title of PR is not following the commitlint conventions, [Semantic Pull Request](https://github.com/zeke/semantic-pull-requests) will complain about it, disallowing your PR to be merged. When your PR is accepted and merged into the master branch, the title of the PR will be recorded as a single commit message which will then added as a single item in [CHANGELOG.md](/CHANGELOG.md).

## Testing

Gosling.js uses [Vitest](https://vitest.dev/) for running tests. To run all of the tests, you can use the command `yarn test`.

## Testing Production Build Using Editor

It is sometimes necessary to test the production build of Gosling.js. This frequently happened to us when we needed to ensure that certain data fetchers, like BAM and VCF, work correctly without errors in a deployed app.
Expand Down
File renamed without changes.
19 changes: 7 additions & 12 deletions editor/html-template.ts
Original file line number Diff line number Diff line change
@@ -1,24 +1,19 @@
import { version as goslingVersion } from '../package.json';
const higlassVersion = '1.11';
const reactVersion = '17';
const pixiVersion = '6';
import { version as higlassVersion } from 'higlass/package.json';

export const getHtmlTemplate = (spec: string) => `
<!DOCTYPE html>
<html>
<head>
<title>Gosling Visualization</title>
<link rel="stylesheet" href="https://unpkg.com/higlass@${higlassVersion}/dist/hglib.css">
<script src="https://unpkg.com/react@${reactVersion}/umd/react.production.min.js"></script>
<script src="https://unpkg.com/react-dom@${reactVersion}/umd/react-dom.production.min.js"></script>
<script src="https://unpkg.com/pixi.js@${pixiVersion}/dist/browser/pixi.min.js"></script>
<script src="https://unpkg.com/higlass@${higlassVersion}/dist/hglib.js"></script>
<script src="https://unpkg.com/gosling.js@${goslingVersion}/dist/gosling.js"></script>
<link rel="stylesheet" href="https://esm.sh/higlass@${higlassVersion}/dist/hglib.css">

</head>
<body>
<div id="gosling-container"/>
<script>
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';
Copy link
Member

@manzt manzt Oct 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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';

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a PR to HiGlass to upgrade Pixi #977

embed(document.getElementById('gosling-container'), ${spec})
</script>
</body>
</html>
Expand Down
33 changes: 0 additions & 33 deletions embed/index.ts

This file was deleted.

4 changes: 4 additions & 0 deletions knip.config.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
etowahadams marked this conversation as resolved.
Show resolved Hide resolved
"entry": ["./src/index.ts", "./editor/index.tsx", "scripts/setup-vitest.js"],
"project": ["**/*.ts", "**/*.tsx"]
}
35 changes: 15 additions & 20 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,21 +8,24 @@
"url": "https://github.com/gosling-lang/gosling.js"
},
"homepage": "https://gosling-lang.github.io/gosling.js/",
"main": "dist/gosling.js",
"main": "dist/gosling.es.js",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"main" applies to both ES module and CJS module entry points. There is no dist/gosling.js created anymore so we point this to gosling.es.js

"module": "dist/gosling.es.js",
"types": "dist/src/index.d.ts",
"files": [
"dist"
],
"type": "module",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My understanding of this is that with the exception of some configs, we want all files to be treated as ESM so we use this option.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That makes sense! I did try upgrading Vite before, but it we're using a plugin (@vitejs/plugin-react-refresh) which requires this version of vite. However, I didn't realize until now that the package is now deprecated in favor of @vitejs/plugin-react.

I'll switch to @vitejs/plugin-react and upgrade vite if that sounds good!

"exports": {
".": "./dist/gosling.es.js",
"./embed": "./dist/embed/index.js"
".": {
"types": "./dist/src/index.d.ts",
"import": "./dist/gosling.es.js"
}
},
"scripts": {
"start": "vite --mode editor",
"start-embed": "vite",
"build": "run-s build-clear build-types build-lib",
"build-lib": "vite build --mode lib && node scripts/build-umd && node scripts/build-embed",
"build-lib": "vite build --mode lib",
"build-types": "tsc --emitDeclarationOnly -p tsconfig.build.json",
"build-editor": "node --max_old_space_size=8192 ./node_modules/vite/bin/vite.js build",
"build-clear": "rm -rf ./dist",
Expand All @@ -34,27 +37,21 @@
"schema": "node scripts/generate-schemas.mjs",
"predeploy": "yarn build-editor; echo \"gosling.js.org\" >> build/CNAME",
"deploy": "gh-pages -d build",
"version": "conventional-changelog -p angular -i CHANGELOG.md -s && git add CHANGELOG.md"
"version": "conventional-changelog -p angular -i CHANGELOG.md -s && git add CHANGELOG.md",
"knip": "knip --config knip.config.json"
etowahadams marked this conversation as resolved.
Show resolved Hide resolved
},
"peerDependencies": {
"pixi.js": "^6.3.0",
"react": "^16.6.3 || ^17.0.0 || ^18.0.0",
"react-dom": "^16.6.3 || ^17.0.0 || ^18.0.0"
},
"sideEffect": false,
"dependencies": {
"@gmod/bam": "^1.1.18",
"@gmod/bbi": "^3.0.1",
"@gmod/bed": "^2.1.2",
"@gmod/gff": "^1.3.0",
"@gmod/tabix": "^1.5.6",
"@gmod/vcf": "^5.0.10",
"@types/bezier-js": "^4.1.0",
"@types/d3": "^7.0.0",
"@types/lodash": "^4.14.151",
"@types/node": "^18.6.2",
"@types/rbush": "^3.0.0",
"@types/uuid": "^8.3.1",
etowahadams marked this conversation as resolved.
Show resolved Hide resolved
"allotment": "^1.19.0",
"bezier-js": "4.0.3",
"buffer": "^6.0.3",
Expand All @@ -75,23 +72,23 @@
"json-stringify-pretty-compact": "^2.0.0",
"jspdf": "^2.3.1",
"lodash-es": "^4.17.21",
"mixwith": "^0.1.1",
"nanoevents": "^7.0.1",
"pubsub-js": "^1.9.3",
"quick-lru": "^6.1.1",
"rbush": "^3.0.1",
"react-grid-layout": "^1.2.5",
"stream-browserify": "^3.0.0",
"threads": "^1.6.4",
"uuid": "^8.3.2"
},
"devDependencies": {
"@testing-library/react": "^10.4.8",
"@testing-library/user-event": "^12.1.1",
"@types/bezier-js": "^4.1.0",
"@types/d3": "^7.0.0",
"@types/d3-drag": "^2.0.0",
"@types/d3-dsv": "^3.0.1",
"@types/d3-request": "^1.0.6",
"@types/d3-selection": "^2.0.0",
"@types/node": "^18.6.2",
"@types/rbush": "^3.0.0",
"@types/uuid": "^8.3.1",
"@types/lodash-es": "^4.17.5",
"@types/pubsub-js": "^1.8.2",
"@types/react": "^18.2.0",
Expand All @@ -104,10 +101,8 @@
"ajv": "^6.12.2",
"c8": "^7.11.2",
"conventional-changelog-cli": "^2.1.1",
"cross-fetch": "^3.1.5",
"d3-drag": "^2.0.0",
"d3-selection": "^2.0.0",
"documentation": "^13.0.2",
"esbuild": "^0.12.25",
"eslint": "^8.19.0",
"eslint-config-prettier": "^8.5.0",
Expand All @@ -116,10 +111,10 @@
"eslint-plugin-react": "^7.30.1",
"fetch-jsonp": "^1.1.3",
"gh-pages": "^3.1.0",
"git-branch-is": "^4.0.0",
"jest-canvas-mock": "^2.3.0",
"jsdom": "^19.0.0",
"jsoncrush": "^1.1.6",
"knip": "^2.30.0",
"npm-run-all": "^4.1.5",
"pixi.js": "^6.3.0",
"prettier": "^2.0.5",
Expand Down
60 changes: 0 additions & 60 deletions scripts/build-embed.js

This file was deleted.

36 changes: 0 additions & 36 deletions scripts/build-umd.js

This file was deleted.

10 changes: 3 additions & 7 deletions scripts/setup-vitest.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,14 +25,10 @@ apis.forEach(api => {
});

beforeAll(() => {
// jsdom doesn't come with a WebCrypto implementation (required for uuid)
global.crypto = {
getRandomValues: function (buffer) {
return randomFillSync(buffer);
}
};
etowahadams marked this conversation as resolved.
Show resolved Hide resolved
// jsdom doesn't come with a `URL.createObjectURL` implementation
global.URL.createObjectURL = () => { return ''; };
global.URL.createObjectURL = () => {
return '';
};
});

afterAll(() => {
Expand Down
2 changes: 1 addition & 1 deletion src/core/gosling-component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { type HiGlassApi, HiGlassComponentWrapper } from './higlass-component-wr
import type { TemplateTrackDef, VisUnitApiData } from '@gosling-lang/gosling-schema';
import type { RequestInit } from '@gosling-lang/higlass-schema';
import React, { useState, useEffect, useMemo, useRef, forwardRef, useCallback, useImperativeHandle } from 'react';
import ResizeSensor from 'css-element-queries/src/ResizeSensor';
import { ResizeSensor } from 'css-element-queries';
import * as gosling from '..';
import { getTheme, type Theme } from './utils/theme';
import { createApi, type GoslingApi } from '../api/api';
Expand Down
Loading
Loading