Skip to content

short circuit depth check if current URL is already sufficient for ResourceInterface.resolveRelativeUrl#897

Merged
thescientist13 merged 1 commit into
masterfrom
enhancement/resource-interface-resolveRelativeUrl-current-depth-check
Mar 5, 2022
Merged

short circuit depth check if current URL is already sufficient for ResourceInterface.resolveRelativeUrl#897
thescientist13 merged 1 commit into
masterfrom
enhancement/resource-interface-resolveRelativeUrl-current-depth-check

Conversation

@thescientist13

@thescientist13 thescientist13 commented Feb 19, 2022

Copy link
Copy Markdown
Member

…race resolveRelativeUrl

Related Issue

N / A

As part of AnalogStudiosRI/www.analogstudios.net#72, noticed that if I had a deep route like
/albums/some-album-name/

Deeply nested paths and relative directories were causing issues, lead to file being loaded like this from the browser

../albums/styles/theme.css

This is more or less what #691 is an open discussion about

This of course can be solved in userland already but requires a conditional with the current ResourceInterface.resolveRelativeUrl because we start slicing and return undefined

const location = this.resolveRelativeUrl(nodeModulesLocation, barePath) || barePath;

However, since the routing will always be dynamic, it would just a bit more ergonomic if resolveRelativeUrl could just help us out regardless of the scenario

const location = this.resolveRelativeUrl(nodeModulesLocation, barePath)

Summary of Changes

  1. Make ResourceInterface.resolveRelativeUrl a little more ergonomic by just return the URL if it actually doesn't need to be resolved

@thescientist13 thescientist13 added enhancement Improve something existing (e.g. no docs, new APIs, etc) CLI labels Feb 19, 2022
@thescientist13 thescientist13 changed the title return if current depth check is already sufficient for resource inte… current URL if depth check is already sufficient for ResourceInteface.resolveRelativeUrl Feb 19, 2022
@thescientist13 thescientist13 changed the title current URL if depth check is already sufficient for ResourceInteface.resolveRelativeUrl current URL if depth check is already sufficient for ResourceInterface.resolveRelativeUrl Feb 19, 2022
@thescientist13 thescientist13 changed the title current URL if depth check is already sufficient for ResourceInterface.resolveRelativeUrl short circuit depth check if current URL is already sufficient for ResourceInterface.resolveRelativeUrl Feb 19, 2022
@thescientist13
thescientist13 force-pushed the enhancement/resource-interface-resolveRelativeUrl-current-depth-check branch from 1c90ba4 to ea26291 Compare March 5, 2022 16:57
@thescientist13
thescientist13 merged commit 02ba0bc into master Mar 5, 2022
@thescientist13
thescientist13 deleted the enhancement/resource-interface-resolveRelativeUrl-current-depth-check branch March 5, 2022 17:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLI enhancement Improve something existing (e.g. no docs, new APIs, etc) v0.24.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant