fix(eslint-plugin-react-components): support ESM imports#36287
fix(eslint-plugin-react-components): support ESM imports#36287KirtiRamchandani wants to merge 1 commit into
Conversation
| @@ -85,3 +89,7 @@ async function applyTransforms(filePath: string, transforms?: Array<Transform>): | |||
| await transform(filePath); | |||
| } | |||
| } | |||
|
|
|||
| function addJsonImportAttributes(code: string): string { | |||
| return code.replace(/from\s+(['"][^'"]+\.json['"])\s*;/g, "from $1 with { type: 'json' };"); | |||
| } | |||
There was a problem hiding this comment.
Thanks for contribution!
I'm hesitant to take the build-executor change right now, it runs over the whole monorepo's output, so the surface is much wider than this one package and carries more risk. I'd prefer to keep fix scoped to eslint-plugin-react-components
We can get the same result purely through this package's .swcrc:
"jsc": {
"baseUrl": ".",
"experimental": { "keepImportAttributes": true }
}
@Hotell wdyt?
There was a problem hiding this comment.
@mainframev hehe, yes, that's neat!
I thought of it before working on this. But I agree the package-local .swcrc fix is lower risk for this PR, but the executor-level fix protects any package that emits Node ESM through this build path and imports JSON. The fixture added in this PR demonstrates that case generically, not only for eslint-plugin-react-components. Without the shared fix, each future package that imports JSON will need to rediscover and copy keepImportAttributes; with the shared fix, the generated ESM contract is enforced once in the build pipeline. What do you say?
I was thinking of overall solution. But yeah, I understand the concern about the executor change, but my reason for putting this in the shared build helper is that the problem is with generated ESM output, not only with this package.
Node requires JSON ESM imports to include with { type: 'json' }, and relative ESM imports need to be fully specified. If the build pipeline can emit output that violates those rules, then any package using this SWC build path can hit the same runtime failure once it imports JSON.
The package-local .swcrc fix would solve this occurrence, but it leaves the same issue open for future packages. The executor-level fix makes the build output contract explicit and tested: when we emit ESM with JSON imports, the output should remain Node-compatible. I agree the blast radius should be controlled, which is why the transform is narrowly scoped to static JSON import statements and covered by workspace-plugin tests. My preference is to keep this because it fixes both the immediate package issue and the underlying build-output issue that caused it.
So this had be a centralized prevention for all future/current packages using the shared SWC executor with JSON imports.
I am up to change this to local and specific to package. Let me know if I need to update it.
Thank you again for the review!
📊 Bundle size report✅ No changes found |
|
Pull request demo site: URL |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -0,0 +1,7 @@ | |||
| { | |||
There was a problem hiding this comment.
🕵🏾♀️ visual changes to review in the Visual Change Report
vr-tests-react-components/CalendarCompat 4 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/CalendarCompat.multiDayView - High Contrast.default.chromium.png | 1236 | Changed |
| vr-tests-react-components/CalendarCompat.multiDayView - Dark Mode.default.chromium.png | 1107 | Changed |
| vr-tests-react-components/CalendarCompat.multiDayView - RTL.default.chromium.png | 495 | Changed |
| vr-tests-react-components/CalendarCompat.multiDayView.default.chromium_1.png | 492 | Changed |
vr-tests-react-components/Menu Converged - submenuIndicator slotted content 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Menu Converged - submenuIndicator slotted content.default.submenus open.chromium.png | 413 | Changed |
vr-tests-react-components/Positioning 2 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Positioning.Positioning end.chromium.png | 618 | Changed |
| vr-tests-react-components/Positioning.Positioning end.updated 2 times.chromium.png | 965 | Changed |
vr-tests-react-components/ProgressBar converged 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/ProgressBar converged.Indeterminate + thickness.default.chromium.png | 54 | Changed |
vr-tests-react-components/Skeleton converged 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/Skeleton converged.Opaque Skeleton with square - Dark Mode.default.chromium.png | 2 | Changed |
vr-tests-react-components/TagPicker 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-react-components/TagPicker.disabled - Dark Mode.chromium.png | 658 | Changed |
vr-tests-web-components/MenuList 3 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/MenuList. - RTL.1st selected.chromium_2.png | 39384 | Changed |
| vr-tests-web-components/MenuList. - RTL.2nd selected.chromium_3.png | 38816 | Changed |
| vr-tests-web-components/MenuList. - RTL.normal.chromium_1.png | 39083 | Changed |
vr-tests-web-components/TextInput 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests-web-components/TextInput. - Dark Mode.normal.chromium_1.png | 288 | Changed |
vr-tests/Callout 3 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/Callout.Top left edge.default.chromium.png | 2212 | Changed |
| vr-tests/Callout.No callout width specified.default.chromium.png | 2143 | Changed |
| vr-tests/Callout.Right bottom edge.default.chromium.png | 3095 | Changed |
vr-tests/react-charting-AreaChart 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/react-charting-AreaChart.Custom Accessibility.default.chromium.png | 11 | Changed |
vr-tests/react-charting-GaugeChart 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/react-charting-GaugeChart.Basic.default.chromium.png | 2 | Changed |
vr-tests/react-charting-MultiStackBarChart 4 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/react-charting-MultiStackBarChart.Basic_Absolute - RTL.default.chromium.png | 343 | Changed |
| vr-tests/react-charting-MultiStackBarChart.Basic_PartToWhole - RTL.default.chromium.png | 343 | Changed |
| vr-tests/react-charting-MultiStackBarChart.Basic_Absolute.default.chromium.png | 359 | Changed |
| vr-tests/react-charting-MultiStackBarChart.Basic_PartToWhole - Dark Mode.default.chromium.png | 363 | Changed |
vr-tests/react-charting-SankeyChart 1 screenshots
| Image Name | Diff(in Pixels) | Image Type |
|---|---|---|
| vr-tests/react-charting-SankeyChart.PlaceHolder - Dark Mode.default.chromium.png | 78 | Changed |
There were 5 duplicate changes discarded. Check the build logs for more information.
2 participants
Fixes #36283.
Problem
The ESM entry for
@fluentui/eslint-plugin-react-componentscan fail when imported through the packageexports.importpath. The builtlib/index.jshad a JSON import without an import attribute, extensionless relative imports, and an unconditionalmodule.exportsassignment.Root Cause
The package source is written for the existing repo toolchain, but the SWC ESM build output was not fully Node ESM-compatible for JSON imports or relative file specifiers. The entry also did not expose a default export for
import plugin from ....Solution
require()shape with a guardedmodule.exportsassignment.with { type: 'json' }for JSON imports, with executor test coverage.Tests
corepack yarn nx run eslint-plugin-react-components:build --excludeTaskDependenciesnode --input-type=module -e "import plugin from '@fluentui/eslint-plugin-react-components'; console.log(plugin.meta.name, typeof plugin.rules, Object.keys(plugin.configs).join(','));"node -e "const plugin = require('@fluentui/eslint-plugin-react-components'); console.log(plugin.meta.name, typeof plugin.rules, Object.prototype.hasOwnProperty.call(plugin, 'default'));"corepack yarn nx run eslint-plugin-react-components:format:checkcorepack yarn nx run eslint-plugin-react-components:test --runInBandcorepack yarn nx run eslint-plugin-react-components:lint --excludeTaskDependenciescorepack yarn nx run eslint-plugin-react-components:type-check --excludeTaskDependenciescorepack yarn nx run workspace-plugin:test --testPathPatterns=tools/workspace-plugin/src/executors/build/executor.spec.ts --runInBandcorepack yarn nx run workspace-plugin:lint --excludeTaskDependenciescorepack yarn nx run workspace-plugin:type-check --excludeTaskDependenciescorepack yarn check:changegit diff --checkNote: Node 24 still prints
MODULE_TYPELESS_PACKAGE_JSONfor the ESM smoke because the package does not declaretype: module. This PR keeps that package boundary unchanged so the existing CommonJS require path continues to return the plugin object.