Repository navigation
feat: Fix interop with cjs-module-lexer - #2377
benasher44 wants to merge 1 commit into
Conversation
951c722 to
23c58ad
Compare
|
Well I got pretty far just with this small change, but now I can't figure out how to get the AMD test to pass… any pointers there? |
|
I also tried without adding a file to re-export from + just copy pasting all of the exports in the default export as named exports, but that has the same issue (seemingly) breaking AMD. |
|
When building babel does give a warning; That's also what seems to be changed in the built files. Apart from the expected additional exports it also returned validator before but now that's part of |
|
No problem. Thanks anyway! Yeah that's what I discovered as well. I did all kinds of inspection there and couldn't figure out a different way to get to the original module object shape it was expecting before :( |
|
Might just be a Babel thing. I know there is an outdated PR from one of the maintainers that tried to overhaul the config (also because we're on old versions of Babel and Rollup); #1869 Maybe a smaller version of that is the way to go, but merging PRs is often quite slow with the current maintainers, see #2376 |
|
It could be, but in my local build, I tried with the latest version of Babel 8, which didn't change anything. |
|
I'll take a look at this while on my dev machine. But this is not a disruptive change like what I've seen in the past. Happy to see this land once all t's are crossed. |
|
@profnandaa any chance you'll have time to look soon? |
|
Bisakah aplikasi crome itu di perketat suapaya kalaw saya main slot engine tidak kalah tolong dibantu |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2377 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 114 115 +1
Lines 2599 2598 -1
Branches 658 659 +1
=========================================
- Hits 2599 2598 -1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
10ac1e5 to
385586d
Compare
90a5a6f to
bd984f0
Compare
Split src/index.js into a thin re-export over a new src/validator-main.js
so the compiled CommonJS keeps statically analyzable named exports. Node
uses cjs-module-lexer to expose named ESM imports from CJS modules; the
previous single-file default-export build hid every export behind a
dynamically assembled object, so `import { isEmail } from 'validator'`
failed while `import validator from 'validator'` worked.
- src/validator-main.js holds the full module body as named exports,
byte-identical to master's index.js modulo the export wrapper
- src/index.js re-exports it and re-exports the namespace as default
- validator-main.js added to package files, .gitignore, and the CI
artifact upload (test jobs download build artifacts)
- build-browser.mjs bundles from a default-only virtual entry, keeping
the browser bundle's UMD surface (window.validator, AMD, CJS) exactly
as before
- test/exports.test.js asserts every runtime export is statically
detectable by cjs-module-lexer, following index.js's re-exports; the
test registers as skipped where the lexer cannot load (Node 8)
0fc527f to
f760902
Compare
|
@profnandaa @WikiRik finally got this working in a backward compatible / sustainable way. mind taking another look? |
This moves the main validator exports to validator-main.js (avoid conflict with top-level validator.js when running build), which then allows both a default and named exports in the index by re-exporting. By doing this, we get a CJS validator that also can be name-imported in node ESM.
Checklist