Skip to content

Fixed asset paths in dev on Windows#315

Closed
dkattan wants to merge 1 commit into
CodinGame:mainfrom
dkattan:hotfix/windows-asset-path-import-meta-resolve
Closed

Fixed asset paths in dev on Windows#315
dkattan wants to merge 1 commit into
CodinGame:mainfrom
dkattan:hotfix/windows-asset-path-import-meta-resolve

Conversation

@dkattan
Copy link
Copy Markdown

@dkattan dkattan commented Jan 13, 2024

In my environment import.meta.url does not maintain the relative path to the extension in node_modules:
image

So I added NPM package import-meta-resolve and used their resolve method by replacing

const resolved = await import.meta.resolve!(path, url.pathToFileURL(args.path));

with

const resolved = await resolve(path, url.pathToFileURL(args.path).toString());

But then I had an issue because import.meta.url starts with file://C:/ causing the pathname to be /C:/node_modules which is obviously not correct

image

This can be mitigated by replacing

const pathname = new URL(req.originalUrl, import.meta.url).pathname;

with

 const pathname = new URL(req.originalUrl, server.resolvedUrls?.local.toString())

Comment thread demo/vite.config.ts
resolve: {
dedupe: ['monaco-editor', 'vscode', ...localDependencies]
}
})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Why is github not able to display a proper diff on this file?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I may have messed up the line endings. I can fix it now, give me a few minutes. I'll rebase while I'm at it.

Copy link
Copy Markdown
Contributor

@CGNonofr CGNonofr Jan 16, 2024

Choose a reason for hiding this comment

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

Did you forget to push? :)

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

I think I got it squared away. Would you mind testing these changes in your build environment to ensure there are no regressions?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

It still doesn't seem to be ok 🤔

@dkattan dkattan force-pushed the hotfix/windows-asset-path-import-meta-resolve branch from 202388a to 069cafa Compare January 18, 2024 13:45
@dkattan dkattan force-pushed the hotfix/windows-asset-path-import-meta-resolve branch from 069cafa to 9d4b2e1 Compare January 18, 2024 14:03
Copy link
Copy Markdown
Contributor

@CGNonofr CGNonofr left a comment

Choose a reason for hiding this comment

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

Seems to still build properly on Linux 👍

Comment thread demo/vite.config.ts
newCode += code.slice(i, match.index)

const path = match[1].slice(1, -1)
const resolved = await resolve(path, url.pathToFileURL(args.path).toString())
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it seems the resolve function is not async

Comment thread demo/package.json
"ansi-colors": "^4.1.3",
"dockerode": "^4.0.2",
"express": "^4.18.2",
"import-meta-resolve": "^4.0.0",
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

it should probably be a dev dependency

@CGNonofr
Copy link
Copy Markdown
Contributor

This plugin was externalized and used in #329

@CGNonofr CGNonofr closed this Jan 26, 2024
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.

2 participants