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

Replace shortid with vendored nanoid #3855

Merged
merged 5 commits into from
Aug 23, 2023
Merged

Replace shortid with vendored nanoid #3855

merged 5 commits into from
Aug 23, 2023

Conversation

cmdcolin
Copy link
Collaborator

@cmdcolin cmdcolin commented Aug 14, 2023

Shortid warns that it is deprecated on install https://www.npmjs.com/package/shortid

This PR switches shortid to nanoid

In order to do so, the codebase of nanoid is vendored (copied) into @jbrowse/core/util/nanoid because installing it from NPM gives tricky to solve issues with pure-ESM especially with jest

These could be worked around in jest probably but vendoring solves 'automatically' for us

xref ai/nanoid#439

@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Aug 14, 2023
@cmdcolin cmdcolin added internal and removed needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) labels Aug 14, 2023
@codecov
Copy link

codecov bot commented Aug 14, 2023

Codecov Report

Merging #3855 (192b8e4) into main (6f1fa62) will increase coverage by 0.01%.
Report is 11 commits behind head on main.
The diff coverage is 64.00%.

@@            Coverage Diff             @@
##             main    #3855      +/-   ##
==========================================
+ Coverage   64.39%   64.41%   +0.01%     
==========================================
  Files         999     1003       +4     
  Lines       29710    29743      +33     
  Branches     7125     7134       +9     
==========================================
+ Hits        19133    19158      +25     
- Misses      10415    10420       +5     
- Partials      162      165       +3     
Files Changed Coverage Δ
packages/app-core/src/JBrowseConfig/index.ts 100.00% <ø> (ø)
packages/core/rpc/WebWorkerRpcDriver.ts 0.00% <0.00%> (ø)
packages/product-core/src/Session/BaseSession.ts 72.41% <ø> (ø)
...ckSelectorWidget/components/tree/HamburgerMenu.tsx 31.34% <0.00%> (ø)
...r-genome-view/src/createModel/createConfigModel.ts 77.77% <ø> (ø)
...r-genome-view/src/createModel/createConfigModel.ts 55.55% <ø> (ø)
...jbrowse-web/src/components/ConfigWarningDialog.tsx 0.00% <0.00%> (ø)
...browse-web/src/components/SessionWarningDialog.tsx 35.71% <0.00%> (ø)
products/jbrowse-web/src/SessionLoader.ts 70.68% <50.00%> (ø)
packages/core/util/nanoid.js 59.25% <59.25%> (ø)
... and 9 more

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@cmdcolin cmdcolin merged commit 92c84aa into main Aug 23, 2023
@cmdcolin cmdcolin deleted the nanoid branch August 23, 2023 14:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant