fix: normalize bundler runtime import paths #1997
Conversation
| @@ -46,23 +46,29 @@ class FederationRuntimePlugin { | |||
|
|
|||
| static getTemplate(runtimePlugins: string[], bundlerRuntimePath?: string) { | |||
| // internal runtime plugin | |||
| const normalizedBundlerRuntimePath = ( | |||
| bundlerRuntimePath || BundlerRuntimePath | |||
| ).replaceAll('\\', '/'); | |||
There was a problem hiding this comment.
Can you use path.normalize here?
There was a problem hiding this comment.
yeah can we look at that? otherwise theres also a normalize webpack path function in this codebase it think, for us to resolve the right webpack.
@2heal1 is that right or should we implement this?
There was a problem hiding this comment.
We did try a path.normalize in the normalizWebpackPath util but it apparently was not working, I'll have another look
There was a problem hiding this comment.
I've just double checked, normalizeWebpackPath isn't used for these paths, they're @module-federation modules not webpack.
path.normalize considers the path as already being valid so makes no changes. We tried some different methods but this is the only way we could generate paths that also work on Windows
There was a problem hiding this comment.
yes, i miss to test the function in Windows, much appreciated for modifying this @jonthomp !
And i need to test the modify can be worked as expect in our codebase. I will test it today.
There was a problem hiding this comment.
@2heal1 happy to help!
Sorry I also missed the remaining Windows fix that we have in a patch. I have now added it to this PR.
On Windows, the line webpackLocationWithDetail.split(':').slice(0, -2)[0] returns C because the path is split into ['C', 'some\path\to\webpack']
|
How about using upath ? const upath = require('upath')
const path = require('path')
const normalizeToPosixPath = (p) =>
upath
.normalizeSafe(path.normalize(p || ''))
const windowsPath = `E:\\work\\app-store\\node_modules\\@module-federation\\enhanced\\dist\\src\\lib\\container\\ContainerExposedDependency.js`;
console.log(normalizeToPosixPath(windowsPath) === 'E:/work/app-store/node_modules/@module-federation/enhanced/dist/src/lib/container/ContainerExposedDependency.js'); |
|
I think we can use upath, this is only in the compiler plugin area and might help against more edge cases between different OS, wont add weight the the build payload |
|
@ScriptedAlchemy, sorryIdidnt get back to this before it got merged, I had to head down and figure out some challenges with our requirements. |
Description
The generated bundler runtime import paths don't currently work on Windows. This PR aims to resolve it by doing two things:
.replaceAll('\\', '/')Related Issue
#1961
Types of changes
Checklist