-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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 resolve base in esbuild loader #1854
Conversation
Closes GH-1821. Related-to: wooorm/xdm@db74ddc
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/mdx/mdx/3oQWbHXM47W2rPQu5Xb5h1NzYTga |
This comment has been minimized.
This comment has been minimized.
packages/esbuild/lib/index.js
Outdated
@@ -13,7 +13,9 @@ | |||
* @typedef {ProcessorOptions & {allowDangerousRemoteMdx?: boolean}} Options | |||
*/ | |||
|
|||
import assert from 'node:assert' | |||
import {promises as fs} from 'fs' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would it make sense for fs
to also have a node:
prefix?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good point, I’ll remove the node:
prefixes.
There was some reason why I didn’t apply them here (they are in xdm). Perhaps due to CRA or Next or so?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm, it’s a bit inconsistent now anyway. I’ve made it more consistent. esbuild and the node loader probably run in a Node version that support it. I’ve “downgraded” one instance of MDX core though.
Closes GH-1821.
Related-to: wooorm/xdm@db74ddc