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

fix(html): fix inconsistency in resolving alias in dev and build mode. #18542

Closed
wants to merge 12 commits into from

Conversation

smeng9
Copy link
Contributor

@smeng9 smeng9 commented Oct 31, 2024

Fixes #17910

@bluwy
Copy link
Member

bluwy commented Nov 4, 2024

The visitor function when traversing shouldn't be async as it'll run slower. We had styleUrl and inlineStyles to avoid that, and if we need to make processNodeUrl async, it should probably follow a similar pattern.

Though about the feature as a whole, I wonder if it's feasible to support it entirely in dev to match build. E.g. do we also want to support specify dependency names like <script type="module" src="my-lib" /> or <link rel="stylesheet" href="normalize.css" />. To align it completely it would have to match the normalizeUrl function in importAnalysis.ts.

My initial hunch is that we don't have to fix #17910. Build has to match dev (and free to extend), but dev doesn't have to match build 🤔

@smeng9
Copy link
Contributor Author

smeng9 commented Nov 4, 2024

That sounds good to make build match dev (and free to extend), but dev doesn't have to match build.

@smeng9 smeng9 closed this Nov 4, 2024
@bluwy
Copy link
Member

bluwy commented Nov 5, 2024

Maybe good to have @sapphi-red's opinion on this too

@sapphi-red
Copy link
Member

I think it'd be nice to make build match to dev too. It's not important than making dev to match build though.

In this case, I think changing the build's behavior to align with dev is also a choice. I hope that won't break anything in ecosystem-ci.

@bluwy
Copy link
Member

bluwy commented Nov 5, 2024

My opinion was actually we don't have to shrink down the build behaviour to the same level as dev. The dev server is where you'd spend most of your time on and becomes the source of truth. Build is an optimization step before deploying, so if it supports something else out of scope of dev, it doesn't matter since you don't realistically develop against build all the time.

We could shrink it down if it's easy, but if not I think it's acceptable to leave it as is too.

@sapphi-red
Copy link
Member

Yeah, I won't say it's necessary. But I'd be open to align them unless the implementation gets much complicated.

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 this pull request may close these issues.

Inconsistent path resolution for references in HTML
3 participants