-
Notifications
You must be signed in to change notification settings - Fork 29.6k
fix: handle raw-loader CSS imports correctly #82095
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
base: canary
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
'use client' | ||
|
||
import React from 'react' | ||
|
||
// This will be a mapping object for all your MDX files | ||
import StuffMdx from './stuff.mdx' | ||
import AnotherMdx from './another.mdx' | ||
// ... import all your MDX files | ||
|
||
const mdxComponents = { | ||
'./stuff.mdx': StuffMdx, | ||
'./another.mdx': AnotherMdx, | ||
// ... map all your imports | ||
} | ||
|
||
export default function MdxLoader({ filePath }) { | ||
const Component = mdxComponents[filePath] | ||
|
||
if (!Component) { | ||
return <div>MDX file not found: {filePath}</div> | ||
} | ||
|
||
return <Component /> | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
'use client' | ||
|
||
import React, { useState, useEffect } from 'react' | ||
|
||
export default function MdxWrapper({ filePath }) { | ||
const [Component, setComponent] = useState(null) | ||
const [error, setError] = useState(null) | ||
|
||
useEffect(() => { | ||
async function loadMdx() { | ||
try { | ||
const module = await import(`${filePath}`) | ||
setComponent(() => module.default) | ||
} catch (err) { | ||
console.error(`Error loading MDX file: ${filePath}`, err) | ||
setError(err) | ||
} | ||
} | ||
|
||
loadMdx() | ||
}, [filePath]) | ||
|
||
if (error) return <div>Error loading MDX content</div> | ||
if (!Component) return <div>Loading...</div> | ||
|
||
return <Component /> | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,11 @@ | ||
import { redirect as nextRedirect } from 'next/dist/client/components/redirect' | ||
|
||
export function redirect(path: string): ReturnType<typeof nextRedirect> { | ||
if (path === '' || path === undefined) { | ||
throw new Error( | ||
'Redirect path cannot be empty. This would cause an infinite loop.' | ||
) | ||
} | ||
|
||
return nextRedirect(path) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,22 @@ | ||
import { Suspense } from 'react' | ||
// import { redirect } from './navigation' - removed | ||
|
||
// Instead of importing dynamically, create an async component | ||
async function MdxContent({ filePath }) { | ||
// This works in a server component | ||
const { default: Content } = await import(`${filePath}`) | ||
return <Content /> | ||
} | ||
|
||
export default function Home() { | ||
// This will now throw an error instead of causing an infinite loop | ||
// redirect(""); | ||
|
||
return ( | ||
<div> | ||
<Suspense fallback={<div>Loading MDX...</div>}> | ||
<MdxContent filePath="./stuff.mdx" /> | ||
</Suspense> | ||
</div> | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
import fs from 'fs' | ||
import path from 'path' | ||
|
||
// This function can be used at build time to generate a mapping of MDX files | ||
export function getMdxFiles(dir) { | ||
const mdxDir = path.join(process.cwd(), dir) | ||
const filenames = fs.readdirSync(mdxDir) | ||
const mdxFiles = filenames.filter((name) => name.endsWith('.mdx')) | ||
|
||
return mdxFiles.map((filename) => ({ | ||
filename, | ||
path: `${dir}/${filename}`, | ||
})) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
/** @type {import('next').NextConfig} */ | ||
const nextConfig = { | ||
webpack: (config) => { | ||
// This is the problematic configuration mentioned in the GitHub issue | ||
// It should work with our fix | ||
config.module.rules.push({ | ||
resourceQuery: /raw/, | ||
type: 'asset/source', | ||
}) | ||
return config | ||
}, | ||
} | ||
|
||
module.exports = nextConfig |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
import React from 'react' | ||
|
||
// Import CSS with raw-loader - this should return raw source, not processed CSS | ||
const rawCss = require('!!raw-loader!./styles.css') | ||
|
||
export default function HomePage() { | ||
return ( | ||
<div> | ||
<h1>Raw Loader Test</h1> | ||
<p> | ||
This page tests that CSS imported with raw-loader returns raw source | ||
instead of processed CSS. | ||
</p> | ||
|
||
<h2>Raw CSS Content:</h2> | ||
<pre | ||
style={{ | ||
backgroundColor: '#f5f5f5', | ||
padding: '10px', | ||
border: '1px solid #ddd', | ||
whiteSpace: 'pre-wrap', | ||
fontSize: '12px', | ||
}} | ||
> | ||
{rawCss} | ||
</pre> | ||
|
||
<h2>Test Results:</h2> | ||
<ul> | ||
<li> | ||
If the CSS content above shows raw CSS (like{' '} | ||
<code> | ||
.test {'{'} color: red; {'}'} | ||
</code> | ||
), the fix is working ✅ | ||
</li> | ||
<li> | ||
If the page has a red background (from the CSS being applied), the fix | ||
is not working ❌ | ||
</li> | ||
</ul> | ||
</div> | ||
) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
/* This CSS file is imported with raw-loader */ | ||
/* If the fix is working, this should be returned as raw text, not applied as styles */ | ||
|
||
body { | ||
background-color: red !important; | ||
color: white !important; | ||
} | ||
|
||
.test { | ||
color: red; | ||
font-weight: bold; | ||
} | ||
|
||
/* This should not be applied to the page when imported with raw-loader */ |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
/* eslint-env jest */ | ||
import { join } from 'path' | ||
import { createNext, FileRef } from 'e2e-utils' | ||
import { fetchViaHTTP } from 'next-test-utils' | ||
|
||
describe('Raw Loader CSS Import', () => { | ||
let next | ||
|
||
beforeAll(async () => { | ||
const files = { | ||
'pages/index.js': new FileRef(join(__dirname, '../pages/index.js')), | ||
'pages/styles.css': new FileRef(join(__dirname, '../pages/styles.css')), | ||
'next.config.js': new FileRef(join(__dirname, '../next.config.js')), | ||
} | ||
|
||
next = await createNext({ | ||
files, | ||
dependencies: { | ||
'raw-loader': '^4.0.2', | ||
}, | ||
}) | ||
}) | ||
|
||
afterAll(async () => { | ||
await next.destroy() | ||
}) | ||
|
||
it('should return raw CSS content instead of applying styles', async () => { | ||
const res = await fetchViaHTTP(next.url, '/') | ||
const html = await res.text() | ||
|
||
// The page should NOT have a red background (which would indicate CSS was applied) | ||
expect(html).not.toContain('background-color: red') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test assertion may be unreliable because it checks for the absence of Consider using a more specific assertion that differentiates between CSS being applied versus displayed as text. For example:
Spotted by Diamond |
||
|
||
// The page should contain the raw CSS content | ||
expect(html).toContain('/* This CSS file is imported with raw-loader */') | ||
expect(html).toContain('body {') | ||
expect(html).toContain('background-color: red !important;') | ||
expect(html).toContain('.test {') | ||
expect(html).toContain('color: red;') | ||
|
||
// The page should show the test instructions | ||
expect(html).toContain('Raw Loader Test') | ||
expect(html).toContain('If the CSS content above shows raw CSS') | ||
}) | ||
}) |
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.
Function parameter is typed as
string
but the runtime check includesundefined
, creating a type inconsistency.View Details
Analysis
The
redirect
function is typed to accept astring
parameter:redirect(path: string)
, but the runtime validation checks for both empty string ANDundefined
:if (path === '' || path === undefined)
.This creates a type inconsistency because:
undefined
to be passed to the function due to thestring
typeundefined
check is unreachable code that serves no purposeundefined
values are expected, the type signature should reflect thatThis could indicate either:
string | undefined
Recommendation
Choose one of these approaches:
Option 1 - If undefined values should be accepted:
Option 2 - If only strings are expected (remove dead code):