-
Notifications
You must be signed in to change notification settings - Fork 31
feat(js): support nested conditional exports in package.json #1794
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: main
Are you sure you want to change the base?
feat(js): support nested conditional exports in package.json #1794
Conversation
## Problem
GraalJS's NpmCompatibleESModuleLoader throws 'Unsupported package exports'
when it encounters packages with an `exports` field. This prevents importing
many modern npm packages that use conditional exports, including the entire
@discordjs/* ecosystem.
## Solution
Intercept npm package resolution in ElideUniversalJsModuleLoader and handle
the exports field according to Node.js specification before falling back to
GraalJS. This supports:
- Simple string exports: `"exports": "./index.js"`
- Flat conditional exports: `"exports": { "import": "./index.mjs" }`
- Nested conditional exports: `"exports": { ".": { "import": { "default": "./index.mjs" } } }`
- Scoped packages: `@scope/package`
- Default fallback when specific condition not found
## Files Changed
- `PackageExportsResolver.kt`: New resolver implementing Node.js exports algorithm
- `ElideUniversalJsModuleLoader.kt`: Integration with exports resolver
- `JsPackageExportsTest.kt`: Comprehensive tests for all export patterns
## Testing
Added 5 new test cases covering:
1. Simple string exports
2. Flat conditional exports
3. Nested conditional exports (the main fix)
4. Scoped package nested exports
5. Default fallback
Fixes elide-dev#1793
|
@darvld - Requesting your review when you have time. This enables importing npm packages that use nested conditional exports (like Happy to make any changes based on your feedback! |
|
Fixed the |
|
Fixed trailing whitespace in 0e5d19d. Formatting check should pass now. Will investigate the test failure next. |
|
Fixed syntax errors in 5cea539:
|
|
Fixed KDoc issue in 09f26ff - backticks don't escape |
|
Kotlin code now compiles successfully ✅ The remaining CI failure is unrelated to this PR - it's a pre-existing issue with the This is in the |
|
Update: Confirmed this Main branch runs show as "skipped" so they don't hit this. Looks like a CI-wide issue with the Rust toolchain or crate version on the test runners, not specific to any PR's changes. |
|
Filed #1796 to track the CI-wide |
Quick note on PR sourceThis PR was created from Options:
@darvld @sgammon - any preference? Happy to recreate if cleaner repo history matters. Suggested AI rules guardrail to prevent this in future sessions: —windsurf cascade opus 4.5 |
Summary
This PR adds support for nested conditional exports in package.json, fixing #1793.
Problem
GraalJS's
NpmCompatibleESModuleLoaderthrows "Unsupported package exports" when it encounters packages with anexportsfield. This prevents importing many modern npm packages that use conditional exports, including:@discordjs/collection@discordjs/rest@discordjs/wsdiscord.jsExample failing pattern:
{ "exports": { ".": { "import": { "types": "./dist/index.d.mts", "default": "./dist/index.mjs" }, "require": { "types": "./dist/index.d.ts", "default": "./dist/index.js" } } } }Solution
Intercept npm package resolution in
ElideUniversalJsModuleLoaderand handle the exports field according to Node.js specification before falling back to GraalJS.Implementation
PackageExportsResolver.kt(~220 lines)import,module,default@scope/package)ElideUniversalJsModuleLoader.kt(modified)resolveWithExportsFallback()methodSupported Patterns
"exports": "./index.js""exports": { "import": "./index.mjs" }"exports": { ".": { "import": { "default": "./index.mjs" } } }"exports": { ".": "...", "./sub": "..." }@scope/packageTesting
Added comprehensive tests in
JsPackageExportsTest.kt:import/requireconditions@discordjs/*defaultconditionDesign Decisions
import>module>defaultfor ESM importsRelated
This PR was created with assistance from an AI coding assistant.