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

Check for existence of window more robustly to allow in SSR or node applications #1811

Merged
merged 3 commits into from
Mar 23, 2021

Conversation

elliothershberg
Copy link
Member

This is a tiny PR takes some of the logic out of #1125 that changes how window is used. This is to help with running JB2 in other contexts than the browser, where window might not be defined.

This is related to #1786 . I'm looking into how our React LGV works inside of a vanilla Next.js app, and have run into issues with window not being defined.

@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Mar 18, 2021
@elliothershberg elliothershberg added bug Something isn't working and removed needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) labels Mar 18, 2021
@rbuels
Copy link
Contributor

rbuels commented Mar 19, 2021

seems pretty straightforward, any reason this is still a draft?

@elliothershberg elliothershberg marked this pull request as ready for review March 19, 2021 15:39
@elliothershberg
Copy link
Member Author

seems pretty straightforward, any reason this is still a draft?

Was just checking to make sure it removed all lines causing the error I was seeing. Yeah, should be good to go.

@codecov
Copy link

codecov bot commented Mar 19, 2021

Codecov Report

Merging #1811 (090942b) into master (cf315fb) will decrease coverage by 0.01%.
The diff coverage is 72.72%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1811      +/-   ##
==========================================
- Coverage   58.60%   58.58%   -0.02%     
==========================================
  Files         459      459              
  Lines       21296    21298       +2     
  Branches     5022     5027       +5     
==========================================
- Hits        12481    12478       -3     
- Misses       8505     8510       +5     
  Partials      310      310              
Impacted Files Coverage Δ
packages/core/util/index.ts 80.99% <40.00%> (-0.83%) ⬇️
packages/core/rpc/BaseRpcDriver.ts 93.75% <100.00%> (ø)
packages/core/rpc/ElectronRpcDriver.ts 3.50% <100.00%> (+1.72%) ⬆️
packages/core/util/calculateStaticBlocks.ts 100.00% <100.00%> (ø)
packages/core/util/io/ElectronLocalFile.ts 3.70% <100.00%> (ø)
packages/core/util/io/ElectronRemoteFile.ts 0.94% <100.00%> (ø)
...inearGenomeView/components/RefNameAutocomplete.tsx 89.36% <0.00%> (-4.26%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cf315fb...090942b. Read the comment docs.

@rbuels rbuels merged commit 7046981 into master Mar 23, 2021
@rbuels rbuels deleted the remove_window branch March 23, 2021 17:14
@cmdcolin cmdcolin changed the title Change usage of window Check for existence of window more robustly to allow in SSR or node applications Mar 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants