Fix relative paths #4594

code-asher · 2021-12-08T20:21:03Z

Since this is for file system paths it feels incorrect to use it on URL paths as they are different in many ways.

Before we relied on the client to resolve the base given to it by the backend against the path. Instead have the client pass that information along so we can resolve it on the backend. This means the client has to do less work.

This is re-used for incremental compilation. Also remove del since that was the only use (and we can use fs.rmdir in the future if we need something like this).

jsjoeio

This means the client has to do less work.

Favorite part of this whole PR 👏

ci/dev/watch.ts

src/node/http.ts

codecov · 2021-12-08T20:47:47Z

Codecov Report

Merging #4594 (70e6399) into main (9d9f3a4) will decrease coverage by 0.18%.
The diff coverage is 38.88%.

@@            Coverage Diff             @@
##             main    #4594      +/-   ##
==========================================
- Coverage   65.80%   65.61%   -0.19%     
==========================================
  Files          30       30              
  Lines        1658     1652       -6     
  Branches      333      330       -3     
==========================================
- Hits         1091     1084       -7     
- Misses        479      481       +2     
+ Partials       88       87       -1

Impacted Files	Coverage Δ
src/common/util.ts	`100.00% <ø> (ø)`
src/node/routes/vscode.ts	`46.93% <0.00%> (ø)`
src/node/routes/logout.ts	`66.66% <25.00%> (+12.12%)`	⬆️
src/node/http.ts	`52.68% <40.00%> (-3.00%)`	⬇️
src/node/routes/login.ts	`84.90% <50.00%> (+3.08%)`	⬆️
src/node/util.ts	`73.13% <100.00%> (ø)`

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 9d9f3a4...70e6399. Read the comment docs.

code-asher added 5 commits December 8, 2021 16:38

Add tests for relativeRoot

a0e928e

Remove path.posix.join

f70a32f

Since this is for file system paths it feels incorrect to use it on URL paths as they are different in many ways.

Rewrite cookie path logic

92de2ae

Before we relied on the client to resolve the base given to it by the backend against the path. Instead have the client pass that information along so we can resolve it on the backend. This means the client has to do less work.

Do not remove out directory before watch

4d5d97f

This is re-used for incremental compilation. Also remove del since that was the only use (and we can use fs.rmdir in the future if we need something like this).

Remove unused function resolveBase

215ddbb

code-asher requested a review from a team as a code owner December 8, 2021 20:21

code-asher changed the title ~~Relative paths~~ Fix relative paths Dec 8, 2021

jsjoeio approved these changes Dec 8, 2021

View reviewed changes

ci/dev/watch.ts Show resolved Hide resolved

src/node/http.ts Outdated Show resolved Hide resolved

src/node/http.ts Show resolved Hide resolved

code-asher added 2 commits December 8, 2021 21:04

fixup! Remove path.posix.join

5a63f49

fixup! Rewrite cookie path logic

70e6399

code-asher merged commit 4b4ec37 into coder:main Dec 8, 2021

code-asher deleted the relative-paths branch December 8, 2021 21:52

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix relative paths #4594

Fix relative paths #4594

code-asher commented Dec 8, 2021 •

edited

Loading

jsjoeio left a comment

codecov bot commented Dec 8, 2021 •

edited

Loading

Fix relative paths #4594

Fix relative paths #4594

Conversation

code-asher commented Dec 8, 2021 • edited Loading

jsjoeio left a comment

Choose a reason for hiding this comment

codecov bot commented Dec 8, 2021 • edited Loading

Codecov Report

code-asher commented Dec 8, 2021 •

edited

Loading

codecov bot commented Dec 8, 2021 •

edited

Loading