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

Remove resolve-url dependency #773

Closed
Balearica opened this issue Jun 2, 2023 · 1 comment · Fixed by #774
Closed

Remove resolve-url dependency #773

Balearica opened this issue Jun 2, 2023 · 1 comment · Fixed by #774

Comments

@Balearica
Copy link
Collaborator

We use a package named resolve-url to transform certain URLs from relative to absolute. It has been marked as depreciated, with the author advising to use URL instead to perform this functionality. While the existing function still works perfectly well, and I see no reason why such a simple piece of code would need to be actively maintained, we should eventually replace the dependency if only so users stop complaining about the depreciation warning.

@Balearica
Copy link
Collaborator Author

It looks like we use the resolveURL function in 3 places.

  1. To set different default options during development in defaultOptions.js
    1. This is only run during development; changing should not impact production code
  2. To convert paths to images to absolute URLs in the browser version of loadImage.js
    1. I do not believe this serves any purpose, and intend on cutting entirely
      1. We use fetch to retrieve images--fetch can handle relative paths without any additional dependency
      2. Images are fetched on the main thread, so there is no issue with workers not knowing what page they are being run on (see below)
  3. To convert corePath, workerPath, and langPath to absolute URLs in the browser version of resolvePaths.js
    1. This is presumably necessary as these paths are sometimes or always retrieved from within a worker
      1. Users likely expect all relative paths to be resolved relative to the main window rather than workers
    2. I believe this one-line function achieves this goal: s => (new URL(s, window.location.href)).href

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant