-
Notifications
You must be signed in to change notification settings - Fork 945
Update all MDX usage to v3 #10106
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: master
Are you sure you want to change the base?
Update all MDX usage to v3 #10106
Changes from 10 commits
e530934
a443eab
8aca136
65fbf7c
6e6ded9
c059a51
5db3ca6
ca366ed
1fe7e0b
891eff5
59c7ef8
fcfb356
cde2250
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Large diffs are not rendered by default.
Large diffs are not rendered by default.
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -6,7 +6,7 @@ Node environment is composed out of the base [React Environment](https://bit.clo | |||||
|
|
||||||
| To use this environment for your components, add it to any of the `variants` in your `workspace.jsonc` file as follows: | ||||||
|
|
||||||
| ```json title="workspace.jsonc" | ||||||
| ```json | ||||||
|
||||||
| ```json | |
| ```json title="workspace.jsonc" |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,10 +2,11 @@ import { join } from 'path'; | |
| import { outputFileSync } from 'fs-extra'; | ||
| import type { Compiler, TranspileFileOutput, TranspileFileParams } from '@teambit/compiler'; | ||
| import type { BuiltTaskResult, BuildContext } from '@teambit/builder'; | ||
| import { compileSync as mdxCompileSync } from '@teambit/mdx.compilers.mdx-transpiler'; | ||
| import minimatch from 'minimatch'; | ||
| import { transpileFileContent as babelTranspileFileContent } from '@teambit/compilation.modules.babel-compiler'; | ||
| import type { TransformOptions } from '@babel/core'; | ||
| import { compileSync as mdxCompileSync } from '@mdx-js/mdx'; | ||
| import { basicMdxOptions } from '@teambit/mdx.modules.mdx-v3-options'; | ||
|
|
||
| export type MDXCompilerOpts = { | ||
| ignoredExtensions?: string[]; | ||
|
|
@@ -33,14 +34,10 @@ export class MDXCompiler implements Compiler { | |
| } | ||
|
|
||
| transpileFile(fileContent: string, options: TranspileFileParams): TranspileFileOutput { | ||
| const afterMdxCompile = mdxCompileSync(fileContent, { | ||
| filepath: options.filePath, | ||
| // this compiler is not indented to compile according to the bit flavour. | ||
| bitFlavour: false, | ||
| }); | ||
| const afterMdxCompile = mdxCompileSync(fileContent, basicMdxOptions); | ||
| const filePathAfterMdxCompile = this.replaceFileExtToJs(options.filePath); | ||
| const afterBabelCompile = babelTranspileFileContent( | ||
| afterMdxCompile.contents, | ||
| afterMdxCompile.toString(), | ||
|
Comment on lines
+37
to
+40
|
||
| { | ||
| rootDir: options.componentDir, | ||
| filePath: filePathAfterMdxCompile, | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,5 +1,23 @@ | ||||||||||||||||||||||||||||||||||||||
| import type { DependencyDetector, FileContext } from '@teambit/dependency-resolver'; | ||||||||||||||||||||||||||||||||||||||
| import { compileSync } from '@teambit/mdx.compilers.mdx-transpiler'; | ||||||||||||||||||||||||||||||||||||||
| import { compileSync } from '@mdx-js/mdx'; | ||||||||||||||||||||||||||||||||||||||
| import { mdxOptions } from '@teambit/mdx.modules.mdx-v3-options'; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| type ImportSpecifier = { | ||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * relative/absolute or module name. e.g. the `y` in the example of `import x from 'y';` | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| fromModule: string; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * is default import (e.g. `import x from 'y';`) | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| isDefault?: boolean; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||||||
| * the name used to identify the module, e.g. the `x` in the example of `import x from 'y';` | ||||||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||||||
| identifier?: string; | ||||||||||||||||||||||||||||||||||||||
| }; | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+4
to
21
|
||||||||||||||||||||||||||||||||||||||
| type ImportSpecifier = { | |
| /** | |
| * relative/absolute or module name. e.g. the `y` in the example of `import x from 'y';` | |
| */ | |
| fromModule: string; | |
| /** | |
| * is default import (e.g. `import x from 'y';`) | |
| */ | |
| isDefault?: boolean; | |
| /** | |
| * the name used to identify the module, e.g. the `x` in the example of `import x from 'y';` | |
| */ | |
| identifier?: string; | |
| }; | |
| import type { ImportSpecifier } from '@mdx-js/mdx'; |
Copilot
AI
Dec 5, 2025
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.
Similar to the doc-reader, accessing output.data.imports without checking if output.data exists could cause runtime errors. Add a null check: const imports = output.data?.imports as ImportSpecifier[] || []; to handle cases where the data property might be undefined.
| const imports = output.data.imports as ImportSpecifier[]; | |
| const imports = output.data?.imports as ImportSpecifier[] || []; |
Copilot
AI
Dec 5, 2025
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.
In MDX v3, the compileSync function returns a VFile object which needs to be converted to a string using .toString() or accessed via .value property. The data.imports property may not exist unless specific plugins are configured to populate it. The expected structure for imports in MDX v3 is different from v1, and without proper plugin configuration, this will likely fail at runtime.
| const imports = output.data.imports as ImportSpecifier[]; | |
| if (!imports) return []; | |
| const imports = Array.isArray(output.data?.imports) ? output.data.imports as ImportSpecifier[] : []; | |
| if (!imports.length) return []; |
| Original file line number | Diff line number | Diff line change | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,13 +1,14 @@ | ||||||||||||
| import type { DocReader } from '@teambit/docs'; | ||||||||||||
| import { Doc } from '@teambit/docs'; | ||||||||||||
| import { compile } from '@teambit/mdx.compilers.mdx-transpiler'; | ||||||||||||
| import { compileSync } from '@mdx-js/mdx'; | ||||||||||||
| import { mdxOptions } from '@teambit/mdx.modules.mdx-v3-options'; | ||||||||||||
|
|
||||||||||||
| export class MDXDocReader implements DocReader { | ||||||||||||
| constructor(private extensions: string[]) {} | ||||||||||||
|
|
||||||||||||
| async read(path: string, contents: Buffer) { | ||||||||||||
| const output = await compile(contents.toString('utf-8'), { filepath: path }); | ||||||||||||
| const metadata = output.getMetadata(); | ||||||||||||
| const output = compileSync(contents.toString('utf-8'), mdxOptions); | ||||||||||||
| const metadata = output.data.frontmatter as any; | ||||||||||||
|
||||||||||||
| const metadata = output.data.frontmatter as any; | |
| const metadata = output.data?.frontmatter as any || {}; |
Copilot
AI
Dec 5, 2025
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.
The MDX compilation result is cast to as any when accessing the frontmatter data. This loses type safety for the metadata extraction.
Consider defining a proper type interface for the MDX compilation output data structure instead of using as any.
Copilot
AI
Dec 5, 2025
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.
In MDX v3, the compileSync function returns a VFile object, not a plain object with a data.frontmatter property. The frontmatter is typically extracted through remark plugins like remark-frontmatter and remark-mdx-frontmatter. Without proper plugin configuration in mdxOptions, accessing output.data.frontmatter may result in undefined at runtime, breaking the metadata extraction.
| const metadata = output.data.frontmatter as any; | |
| // Ensure mdxOptions includes remark-frontmatter and remark-mdx-frontmatter plugins for frontmatter extraction. | |
| const metadata = (output.data && output.data.frontmatter) ? output.data.frontmatter : {}; |
| Original file line number | Diff line number | Diff line change | ||||||||
|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -172,7 +172,7 @@ export class ReactEnv | |||||||||
| jest: jestModulePath || require.resolve('jest'), | ||||||||||
| config, | ||||||||||
| }, | ||||||||||
| { logger: this.logger, worker } | ||||||||||
| { logger: this.logger, worker, devFiles: this.devFiles } | ||||||||||
| ); | ||||||||||
|
|
||||||||||
| return tester; | ||||||||||
|
|
@@ -205,7 +205,8 @@ export class ReactEnv | |||||||||
| jest: jestModulePath || require.resolve('jest'), | ||||||||||
| config, | ||||||||||
| }, | ||||||||||
| { logger: this.logger, worker } | ||||||||||
| { logger: this.logger, worker } as any | ||||||||||
| // { logger: this.logger, worker, devFiles: this.devFiles }, | ||||||||||
|
||||||||||
| { logger: this.logger, worker } as any | |
| // { logger: this.logger, worker, devFiles: this.devFiles }, | |
| { logger: this.logger, worker, devFiles: this.devFiles } |
Outdated
Copilot
AI
Dec 5, 2025
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.
Commented-out code should be removed rather than left in the codebase. The commented line appears to be the correct implementation (including devFiles: this.devFiles), while the active line uses a type assertion (as any) to bypass type checking. Either the commented code should be uncommented and the type issue fixed properly, or it should be removed entirely if it's no longer needed.
| { logger: this.logger, worker } as any | |
| // { logger: this.logger, worker, devFiles: this.devFiles }, | |
| { logger: this.logger, worker, devFiles: this.devFiles }, |
Outdated
Copilot
AI
Dec 5, 2025
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.
Similar to line 209, there's commented-out code with uncertainty about the devFiles parameter. This should be resolved - either include the parameter or remove the commented code completely. Having commented code in production suggests incomplete migration.
| // { logger: this.logger, worker, devFiles: this.devFiles }, |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -467,7 +467,7 @@ export class ReactMain { | |
| ); | ||
| const appType = new ReactAppType('react-app', reactEnv, logger, dependencyResolver); | ||
| const react = new ReactMain(reactEnv, envs, application, appType, dependencyResolver, logger); | ||
| graphql.register(() => reactSchema(react)); | ||
| graphql.register(() => reactSchema(react) as any); | ||
|
||
| envs.registerEnv(reactEnv); | ||
| if (generator) { | ||
| const envContext = new EnvContext(ComponentID.fromString(ReactAspect.id), loggerMain, workerMain, harmony); | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -12,8 +12,10 @@ import { postCssConfig } from './postcss.config'; | |||||||||||||||||||||||||
| // TODO: remove it once we can set policy from component to component then set it via the component.json | ||||||||||||||||||||||||||
| import '@teambit/react.babel.bit-react-transformer'; | ||||||||||||||||||||||||||
| // Make sure the mdx-loader is a dependency | ||||||||||||||||||||||||||
| import '@teambit/mdx.modules.mdx-loader'; | ||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||
| // eslint-disable-next-line import/no-unresolved | ||||||||||||||||||||||||||
|
||||||||||||||||||||||||||
| // eslint-disable-next-line import/no-unresolved |
Copilot
AI
Dec 4, 2025
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.
Incorrect loader order for MDX processing
Webpack loaders execute from bottom to top in the use array. The current order has:
@mdx-js/loader(executed second)@teambit/mdx.modules.mdx-pre-loader(executed first)
This means the pre-loader runs before the main MDX loader, but based on the naming convention, a "pre-loader" should process the file before it's passed to the main loader.
If this is intentional and the pre-loader should run first, consider renaming it to "mdx-post-loader" to clarify the execution order. Otherwise, reverse the order:
{
loader: require.resolve('@teambit/mdx.modules.mdx-pre-loader'),
},
{
loader: require.resolve('@mdx-js/loader'),
options: mdxOptions,
},| loader: require.resolve('@mdx-js/loader'), | |
| options: mdxOptions, | |
| }, | |
| { | |
| loader: require.resolve('@teambit/mdx.modules.mdx-pre-loader'), | |
| }, | |
| loader: require.resolve('@teambit/mdx.modules.mdx-pre-loader'), | |
| }, | |
| { | |
| loader: require.resolve('@mdx-js/loader'), | |
| options: mdxOptions, | |
| }, |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -5,6 +5,7 @@ import { ComponentID } from '@teambit/component-id'; | |
| // TODO: remove it once we can set policy from component to component then set it via the component.json | ||
| import '@teambit/react.babel.bit-react-transformer'; | ||
| import ReactRefreshWebpackPlugin from '@pmmmwh/react-refresh-webpack-plugin'; | ||
| import { mdxOptions } from '@teambit/mdx.modules.mdx-v3-options'; | ||
|
|
||
| const matchNothingRegex = 'a^'; | ||
|
|
||
|
|
@@ -71,7 +72,11 @@ export default function (workDir: string, envId: string): Configuration { | |
| }, | ||
| }, | ||
| { | ||
| loader: require.resolve('@teambit/mdx.modules.mdx-loader'), | ||
| loader: require.resolve('@mdx-js/loader'), | ||
| options: mdxOptions, | ||
| }, | ||
| { | ||
| loader: require.resolve('@teambit/mdx.modules.mdx-pre-loader'), | ||
| }, | ||
|
Comment on lines
74
to
80
|
||
| ], | ||
| }, | ||
|
|
||
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.
The removal of
live=truefrom the code block may affect functionality. Thelive=trueattribute typically indicates that the code should be rendered as an interactive example. Verify that this functionality is still supported in MDX v3, or if an alternative approach is needed to maintain the live code preview feature.