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

Restore ability to load plugins from relative URL #2563

Merged
merged 2 commits into from
Dec 9, 2021

Conversation

garrettjstevens
Copy link
Collaborator

Fixes #2562

This passes the window location as a second argument to the URL constructor. If a plugin definition has a full URL, the second argument gets ignored, but if a plugin definition has a relative URL it is resolved relative to the window location. Examples of how the resolution works are here.

It also logs an error message with the name of the failed URL since that's not included in the native error message.

@garrettjstevens garrettjstevens self-assigned this Dec 7, 2021
@github-actions github-actions bot added the needs label triage Needs a label to show in changelog (breaking, enhancement, bug, documentation, or internal) label Dec 7, 2021
@codecov
Copy link

codecov bot commented Dec 7, 2021

Codecov Report

Merging #2563 (91773fa) into main (cbfcd97) will decrease coverage by 0.00%.
The diff coverage is 0.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2563      +/-   ##
==========================================
- Coverage   60.86%   60.86%   -0.01%     
==========================================
  Files         547      547              
  Lines       25727    25734       +7     
  Branches     6078     6076       -2     
==========================================
+ Hits        15659    15663       +4     
- Misses       9743     9746       +3     
  Partials      325      325              
Impacted Files Coverage Δ
packages/core/PluginLoader.ts 17.11% <0.00%> (-1.16%) ⬇️
packages/core/util/layouts/GranularRectLayout.ts 87.87% <0.00%> (+0.43%) ⬆️
packages/core/data_adapters/BaseAdapter.ts 73.87% <0.00%> (+0.90%) ⬆️
...lignments/src/BamAdapter/BamSlightlyLazyFeature.ts 79.24% <0.00%> (+1.88%) ⬆️

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 cbfcd97...91773fa. Read the comment docs.

@garrettjstevens garrettjstevens 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 Dec 8, 2021
)
} catch (error) {
console.error(`Error parsing URL: ${pluginDefinition.cjsUrl}`)
throw error
Copy link
Collaborator

Choose a reason for hiding this comment

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

maybe

throw new Error(`Error parsing URL: ${pluginDefinition.cjsUrl}`)

?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I thought about that, but wanted to keep the original error in case it had any relevant information. I guess that's not likely, though, so maybe I can log the original error and then throw the custom error since that's what users will see in the UI.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍

@cmdcolin cmdcolin merged commit 2752837 into main Dec 9, 2021
@cmdcolin cmdcolin deleted the 2562_relative_plugin_url branch December 9, 2021 01:00
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.

No longer possible to load plugins by relative URL?
2 participants