-
Notifications
You must be signed in to change notification settings - Fork 2.1k
ci: checks for eslint-config-turbo interface
#10794
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Contributor
|
The latest updates on your projects. Learn more about Vercel for GitHub.
1 Skipped Deployment
|
eslint-config-custom interface
eslint-config-custom interfaceeslint-config-turbo interface
anthonyshew
commented
Aug 25, 2025
|
|
||
| module.exports = config; | ||
| // eslint-disable-next-line import/no-default-export -- ESLint convention is a default export | ||
| export default config; |
Contributor
Author
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.
bunchee wasn't converting module.exports to export default for the ESM entrypoints. If I make it export default, it downlevels to CJS correctly.
chris-olszewski
approved these changes
Aug 25, 2025
anthonyshew
added a commit
that referenced
this pull request
Aug 29, 2025
### Description Similar to #10794, adding in some sanity checks before we do some refactors to improve these packages. ### Testing Instructions CI
1 task
anthonyshew
added a commit
that referenced
this pull request
Sep 29, 2025
Fixes #10901 In PR #10794, we changed from module.exports to export default, which caused bunchee to generate exports.default = config instead of module.exports = config in the CommonJS build. ESLint v8 with legacy .eslintrc.* configuration expects shareable configs to export directly via module.exports. When it encounters exports.default, it interprets 'default' as an unexpected top-level configuration property, resulting in the error: 'Unexpected top-level property "default"' This fix reverts to module.exports for the main entry point to restore ESLint v8 compatibility, while maintaining ESLint v9 flat config support through the ./flat export path. Testing: - Verified publint --strict passes - Verified attw type checking passes - Confirmed compiled output uses module.exports - Both CJS require() and ESM import() work correctly
anthonyshew
added a commit
that referenced
this pull request
Oct 6, 2025
…ity (#10902) ### Description Fixes #10901 In PR #10794, we changed from `module.exports` to `export default`, which caused bunchee to generate `exports.default = config` instead of `module.exports = config` in the CommonJS build. ESLint v8 with legacy `.eslintrc.*` configuration expects shareable configs to export directly via `module.exports`. When it encounters `exports.default`, it interprets `'default'` as an unexpected top-level configuration property, resulting in the error: ``` Unexpected top-level property "default" ``` ### Changes 1. **`packages/eslint-config-turbo/src/index.ts`** - Reverted from `export default config;` to `module.exports = config;` - Ensures compiled CommonJS output properly exports the config object directly 2. **`packages/eslint-config-turbo/package.json`** - Removed the `module` field (no longer generating ESM for main entry) - Updated `exports[".'"]` to point both `import` and `require` to the CommonJS build - Maintains compatibility while ensuring ESLint v8 can properly load the configuration ### Why This Fixes The Problem - **Before**: Using `export default` caused bunchee to generate `exports.default = config;` - **After**: Using `module.exports` generates `module.exports = config;` which is what ESLint v8 expects - ESLint v9 flat config support is maintained through the separate `./flat` export path ### Testing Instructions 1. Build the package: `pnpm --filter=eslint-config-turbo build` 2. Run package validation: `pnpm --filter=eslint-config-turbo package:lint` 3. Run type checking: `pnpm --filter=eslint-config-turbo package:types` 4. Verify compiled output: `cat packages/eslint-config-turbo/dist/cjs/index.js` (should show `module.exports = config;`) All checks pass: - ✅ `publint --strict` - ✅ `attw --profile node16 --pack` - ✅ `pnpm lint` - ✅ Correct CommonJS output ### Compatibility - ✅ **ESLint v8 (legacy config)**: Uses `require('eslint-config-turbo')` - now works correctly - ✅ **ESLint v9 (flat config)**: Uses `import turboConfig from 'eslint-config-turbo/flat'` - continues to work - ✅ **Modern bundlers**: Can import from either path CLOSES TURBO-4861 --------- Co-authored-by: pauloZion <paulozionpazi@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Because JavaScript packaging is so finnicky, I'm thinking its a good idea to make sure our interfaces stay ecosystem compliant. PRs like #10747 and #10754 have potential to break things, so its nice to have some sanity checking. (I haven't checked yet if those PRs specifically are right, but they made me think about adding some tooling like this.)
Testing Instructions
CI