Skip to content
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

feat: reexport commonjs #87

Merged
merged 8 commits into from
Oct 23, 2023
Merged

feat: reexport commonjs #87

merged 8 commits into from
Oct 23, 2023

Conversation

underfin
Copy link
Contributor

@underfin underfin commented Oct 21, 2023

Description

The pr is will implement some case for reexport commonjs. The esbuild example see here.

  • implement reexport star commonjs
// index.js
import { a } from './foo'
console.log(a)
// foo.js
export * from './cjs'
// .cjs
module.exports = 1

// compiled to
var require_cjs = __commonJS(xx);
var foo_ns = {};
__reExport(foo_ns, __toEsm(require_cjs()))
console.log(foo_ns.a)
  • implement reexport named declaration from commonjs
// index.js
import { a } from './foo'
console.log(a)
// foo.js
export { a } from './cjs'
// .cjs
module.exports = 1

// compiled to
var require_cjs = __commonJS(xx);
var foo_ns = {
   a() => cjs_ns.a
};
var cjs_ns = __toEsm(require_cjs())
console.log(foo_ns.a)
  • implement reexport named declaration from commonjs at entry
// index.js
export { a } from './cjs'
// .cjs
module.exports = 1

// compiled to
var require_cjs = __commonJS(xx);
var cjs_ns = __toEsm(require_cjs())
let a  = cjs_ns.a
export { a }
  • refactor import from commonjs
// index.js
import { a } from './cjs'
console.log(a)
// .cjs
module.exports = 1

// Before main brach compiled to
var require_cjs = __commonJS(xx);

var cjs_ns = __toEsm(require_cjs())
let a = cjs_ns.a
console.log(a)

// The pr compiled to
var require_cjs = __commonJS(xx);

var cjs_ns = __toEsm(require_cjs())
console.log(cjs_ns.a)

Here followed esbuild result and make sure if the symbol not found error happend at real position.

Implement Detail

  1. add Resolution::Runtime to mark the symbol will being resolved for runtime at linker imports/exports. It worked for export star from commonjs and import symbol from commonjs.
  • If import a symbol is not found from importee but the importee has export star from commonjs, skip union symbols for it and add it to UnresolvedSymbols and will render its reference.
  • If import a symbol from commonjs, skip union symbols for it and add it to UnresolvedSymbols and will render its reference.

The UnresolvedSymbols will record the symbol come from importee namespace symbol and original name. And it codgen a to _ns.a.

  1. Add ResolvedExport::Runtime to mark the symbol reexported form commonjs, eg export { a} from 'cjs', it will render the resolved_exports to a() => cjs_ns.a

related #36.

Test Plan

Added.


@underfin
Copy link
Contributor Author

!bench

@github-actions
Copy link

Benchmark Results

group                            baseline                               pr
-----                            --------                               --
rolldown benchmark/threejs       1.04     62.2±1.45ms        ? ?/sec    1.00     59.8±1.60ms        ? ?/sec
rolldown benchmark/threejs10x    1.00   628.2±17.84ms        ? ?/sec    1.02   638.5±53.78ms        ? ?/sec

if importer.is_entry {
Some(symbol_ref)
// FIXME: this is wrong, should be generate local symbol for runtime symbol
// Some(importer.generate_local_symbol(&mut self.graph.symbols, *exported))
Copy link
Contributor Author

@underfin underfin Oct 21, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The rust ownership can't get mutable refernece of module at same,i will create linker module struct to avoid it at next pr.

@underfin underfin merged commit 20271ad into main Oct 23, 2023
4 checks passed
@underfin underfin deleted the re-export-commonjs-named branch October 23, 2023 12:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants