这是indexloc提供的服务,不要输入任何密码
Skip to content

Conversation

@anthonyshew
Copy link
Contributor

@anthonyshew anthonyshew commented Aug 25, 2025

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

@anthonyshew anthonyshew requested a review from a team as a code owner August 25, 2025 04:52
@turbo-orchestrator turbo-orchestrator bot added area: ci pkg: turbo-eslint eslint-config-turbo and eslint-plugin-turbo labels Aug 25, 2025
@vercel
Copy link
Contributor

vercel bot commented Aug 25, 2025

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Preview Comments Updated (UTC)
examples-basic-web Ready Ready Preview Comment Aug 25, 2025 5:10am
examples-designsystem-docs Ready Ready Preview Comment Aug 25, 2025 5:10am
examples-gatsby-web Ready Ready Preview Comment Aug 25, 2025 5:10am
examples-kitchensink-blog Ready Ready Preview Comment Aug 25, 2025 5:10am
examples-nonmonorepo Ready Ready Preview Comment Aug 25, 2025 5:10am
examples-svelte-web Ready Ready Preview Comment Aug 25, 2025 5:10am
examples-tailwind-web Ready Ready Preview Comment Aug 25, 2025 5:10am
examples-vite-web Ready Ready Preview Comment Aug 25, 2025 5:10am
1 Skipped Deployment
Project Deployment Preview Comments Updated (UTC)
turbo-site Skipped Skipped Aug 25, 2025 5:10am

@anthonyshew anthonyshew changed the title ci: checks for our JavaScript package interfaces ci: checks for eslint-config-custom interface Aug 25, 2025
@anthonyshew anthonyshew changed the title ci: checks for eslint-config-custom interface ci: checks for eslint-config-turbo interface Aug 25, 2025

module.exports = config;
// eslint-disable-next-line import/no-default-export -- ESLint convention is a default export
export default config;
Copy link
Contributor Author

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.

@anthonyshew anthonyshew merged commit 2b1b6c3 into main Aug 25, 2025
70 of 74 checks passed
@anthonyshew anthonyshew deleted the shew/d1e28 branch August 25, 2025 14:13
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
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

Labels

area: ci pkg: turbo-eslint eslint-config-turbo and eslint-plugin-turbo

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants