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

Properly resolve relative paths in CSS imports #527

Merged
merged 4 commits into from
Mar 18, 2024

Conversation

MatthewCash
Copy link
Contributor

Currently, WebCord cannot resolve relative imports because it simply constructs a new URL() with the relative URL which will fail.

This change passes context about the current file to parseImports and fetchOrRead to prepend the previous URL's directory name if creating a URL fails.

Copy link
Owner

@SpacingBat3 SpacingBat3 left a comment

Choose a reason for hiding this comment

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

Thanks to contributing to WebCord!

I overall think the code introduces a lot of redundancy, I won't pull this until either you or I will resolve them. Also, it touches some irrelevant pieces of the code [comparing] to the overall topic of this PR, I guess explaining your approach a bit more and why you haven't considered the others would help me understand your intentions.

sources/code/main/modules/extensions.ts Outdated Show resolved Hide resolved
sources/code/main/modules/extensions.ts Outdated Show resolved Hide resolved
sources/code/main/modules/extensions.ts Outdated Show resolved Hide resolved
sources/code/main/modules/extensions.ts Outdated Show resolved Hide resolved
@SpacingBat3 SpacingBat3 added the type:bug Something isn't working label Mar 12, 2024
@MatthewCash MatthewCash force-pushed the resolve-relative-imports branch from 3b5e51d to e4beaf6 Compare March 13, 2024 00:31
@MatthewCash MatthewCash force-pushed the resolve-relative-imports branch from e4beaf6 to 4148b5c Compare March 13, 2024 00:38
@SpacingBat3
Copy link
Owner

You don't have to force-push PR on changes to have a specific history, in fact it might be better to commit on top of the another. As long as things are around single thing to be implemented, I can always squash your commits later, and having the history of changes done to this PR is for sure useful if we consider to go back to how things were before (isn't that main purpose of git?)

@SpacingBat3 SpacingBat3 self-requested a review March 18, 2024 00:06
Copy link
Owner

@SpacingBat3 SpacingBat3 left a comment

Choose a reason for hiding this comment

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

You still need to resolve some things before I will be willing to pull it (I've made new comments around that).

@SpacingBat3 SpacingBat3 self-requested a review March 18, 2024 02:31
Copy link
Owner

@SpacingBat3 SpacingBat3 left a comment

Choose a reason for hiding this comment

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

LGTM 🎉.

@SpacingBat3 SpacingBat3 merged commit 83a912d into SpacingBat3:master Mar 18, 2024
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants