fix(dev-server-rollup): fix rollup adapter virtual modules resolution for windows environments

[giamir]

Please refer to #2078 for full context.

[changeset-bot]

🦋 Changeset detected

Latest commit: ab27e50

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@web/dev-server-rollup	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[giamir]

@LarsDenBakker I would really appreciate a review on this one-liner PR.
It would help to give WTR a chance to be adopted in Stack Overflow's Design System (for us supporting tests in windows environments is crucial).
Thanks in advance and let me know if there is anything I can do to support. 🙂

[giamir]

@peschee @43081j @Westbrook Apologies in advance for tagging more maintainers.
Please do let me know if there is a better process to receive support.

I would appreciate if someone could take a look at this one-liner PR (bug fix for Windows).

Highly appreciate your community efforts to maintain this project.
Thanks and let me know if I can provide any support.

[43081j]

i wouldn't actually solve this by stripping the null byte, that exists to tell the internals that its not a real path (i.e. it is virtual, handled by a plugin).

in both unix and windows, it shouldn't be isAbsolute because of that.

the bit that's tripping you up here is that:

C:\foo\bar

is parsed as a valid URL - {scheme: 'C', path: '\foo\bar'}

whoever wrote that condition you linked to probably hoped/assumed paths would fail to parse in both OSes.

that should probably be modified to take into account that windows paths can result in valid URLs

i suppose we could check that it is null or has a non-http(s) scheme

[giamir]

Thanks for the prompt response @43081j, I have adjusted the PR based off your suggestion.

Could you have a look? It is still a one-liner after all. 🙂

[giamir]

@43081j I would appreciate if you or another maintainer could help me to get this small fix merged and released. How can I support? Thanks in advance.

[43081j]

makes sense to me.

probably need @daKmoR or @LarsDenBakker to approve/merge

meanwhile, if its possible to add a test for this case, that'd be super helpful

[giamir]

Sorry to ping again in here.
Can @daKmoR, @LarsDenBakker, @Westbrook or @thepassle help me to get this small change merged?
It would be highly appreciated. Thanks in advance!