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.

Sign up for GitHub

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

Jump to bottom

Fix resolve base in esbuild loader #1854

Merged
merged 3 commits into from Dec 30, 2021
Merged

Fix resolve base in esbuild loader #1854

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
b9533d8
Fix resolve base in esbuild loader
wooorm Dec 4, 2021
67a48d0
Fix erbium
wooorm Dec 4, 2021
e791c1f
More consistent `node:` prefix use
wooorm Dec 30, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
17 changes: 15 additions & 2 deletions packages/esbuild/lib/index.js
View file
Open in desktop
Expand Up @@ -13,7 +13,9 @@
* @typedef {ProcessorOptions & {allowDangerousRemoteMdx?: boolean}} Options
*/

import assert from 'node:assert'
import {promises as fs} from 'fs'
Copy link
Member

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?

Copy link
Member Author

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?

Copy link
Member Author

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.

import path from 'node:path'
import process from 'process'
import {URL} from 'url'
import got from 'got'
Expand Down Expand Up @@ -49,6 +51,7 @@ export function esbuild(options = {}) {
const filter = extnamesToRegex(extnames)
/* eslint-disable-next-line security/detect-non-literal-regexp */
const filterHttp = new RegExp('^https?:\\/{2}.+' + filter.source)
const http = /^https?:\/{2}/
const filterHttpOrRelative = /^(https?:\/{2}|.{1,2}\/).*/

if (allowDangerousRemoteMdx) {
Expand Down Expand Up @@ -205,9 +208,19 @@ export function esbuild(options = {}) {
})
}

// Safety check: the file has a path, so there has to be a `dirname`.
assert(file.dirname, 'expected `dirname` to be defined')

// V8 on Erbium.
/* c8 ignore next 2 */
return {contents: value, errors, warnings, resolveDir: p.cwd()}
/* c8 ignore next 9 */
return {
contents: value,
errors,
warnings,
resolveDir: http.test(file.path)
? p.cwd()
: path.resolve(file.cwd, file.dirname)
}
}
}
}
45 changes: 41 additions & 4 deletions packages/esbuild/test/index.test.js
View file
Open in desktop
Expand Up @@ -45,6 +45,45 @@ test('@mdx-js/esbuild', async () => {
await fs.unlink(new URL('./esbuild.mdx', import.meta.url))
await fs.unlink(new URL('./esbuild.js', import.meta.url))

// Resolve directory.
await fs.writeFile(
new URL('./esbuild-resolve.mdx', import.meta.url),
'import Content from "./folder/file.mdx"\n\n<Content/>'
)
await fs.mkdir(new URL('./folder', import.meta.url))
await fs.writeFile(
new URL('./folder/file.mdx', import.meta.url),
'import {data} from "./file.js"\n\n{data}'
)
await fs.writeFile(
new URL('./folder/file.js', import.meta.url),
'export const data = 0.1'
)
await esbuild.build({
bundle: true,
define: {'process.env.NODE_ENV': '"development"'},
entryPoints: [
fileURLToPath(new URL('./esbuild-resolve.mdx', import.meta.url))
],
outfile: fileURLToPath(new URL('./esbuild-resolve.js', import.meta.url)),
format: 'esm',
plugins: [esbuildMdx()]
})
/** @type {MDXContent} */
Content =
/* @ts-expect-error file is dynamically generated */
(await import('./esbuild-resolve.js')).default // type-coverage:ignore-line

assert.equal(
renderToStaticMarkup(React.createElement(Content)),
'0.1',
'should compile'
)

await fs.unlink(new URL('./esbuild-resolve.mdx', import.meta.url))
await fs.unlink(new URL('./esbuild-resolve.js', import.meta.url))
await fs.rmdir(new URL('./folder/', import.meta.url), {recursive: true})

// Markdown.
await fs.writeFile(new URL('./esbuild.md', import.meta.url), '\ta')

Expand Down Expand Up @@ -337,8 +376,6 @@ test('@mdx-js/esbuild', async () => {

await fs.unlink(new URL('./esbuild-warnings.mdx', import.meta.url))

console.log('\nnote: the preceding errors and warnings are expected!\n')

await fs.writeFile(
new URL('./esbuild-plugin-crash.mdx', import.meta.url),
'# hi'
Expand Down Expand Up @@ -466,7 +503,7 @@ test('@mdx-js/esbuild', async () => {
// Remote markdown.
await fs.writeFile(
new URL('./esbuild-with-remote-md.mdx', import.meta.url),
'import Content from "https://raw.githubusercontent.com/wooorm/xdm/main/test/files/md-file.md"\n\n<Content />'
'import Content from "https://raw.githubusercontent.com/mdx-js/mdx/main/packages/esbuild/test/files/md-file.md"\n\n<Content />'
)

await esbuild.build({
Expand Down Expand Up @@ -498,7 +535,7 @@ test('@mdx-js/esbuild', async () => {
// Remote MDX importing more markdown.
await fs.writeFile(
new URL('./esbuild-with-remote-mdx.mdx', import.meta.url),
'import Content from "https://raw.githubusercontent.com/wooorm/xdm/main/test/files/mdx-file-importing-markdown.mdx"\n\n<Content />'
'import Content from "https://raw.githubusercontent.com/mdx-js/mdx/main/packages/esbuild/test/files/mdx-file-importing-markdown.mdx"\n\n<Content />'
)

await esbuild.build({
Expand Down