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

Deno bundler circular imports error #3825

Closed
srackham opened this issue Jan 30, 2020 · 2 comments · Fixed by #3965
Closed

Deno bundler circular imports error #3825

srackham opened this issue Jan 30, 2020 · 2 comments · Fixed by #3965

Comments

@srackham
Copy link

This minimal Gist contains a circular dependency. It is valid TypeScript and it executes fine with the Deno run command but the bundled JavaScript code throws an error:

m1.ts

import * as M2 from './m2.ts'

M2.f2()

export function f1() { }

m2.ts

import * as M1 from './m1.ts'

console.log("M1:",M1)
M1.f1()

export function f2() { }

Here's what happens when m1.ts is run and when m1.ts is bundled and run:

$ deno --version
deno 0.31.0
v8 8.1.108
typescript 3.7.2

$ deno run https://gist.githubusercontent.com/srackham/dd1f86ece0c6e2166506de3de05c5607/raw/8c3f483c5015aeaba7021f47129ab00225d298eb/m1.ts
M1: { f1: [Function: f1] }

$ deno bundle https://gist.githubusercontent.com/srackham/dd1f86ece0c6e2166506de3de05c5607/raw/8c3f483c5015aeaba7021f47129ab00225d298eb/m1.ts m1.js
Bundling "https://gist.githubusercontent.com/srackham/dd1f86ece0c6e2166506de3de05c5607/raw/8c3f483c5015aeaba7021f47129ab00225d298eb/m1.ts"
Emitting bundle to "m1.js"
4.2 kB emitted.

$ deno run m1.js 
M1: { default: {} }
error: Uncaught TypeError: M1.f1 is not a function
► file:///home/srackham/local/projects/rimu-deno/m1.js:135:8

135     M1.f1();
           ^

Issue #653 suggests that circular dependencies should work.

See also Bundling V2 #2475.

@kitsonk
Copy link
Contributor

kitsonk commented Jan 30, 2020

Thanks for the test case... I will have to take a deeper look, but it is possible that we have a defect in the instantiation.

@srackham
Copy link
Author

The error occurs because:

  1. getExports() is called recursively a second time for module M1 before the first call has completed.

  2. Because the M1 factory property has been deleted the second call assumes that a previous call has resolved M1 exports and returns the as yet unpopulated exports object.

  3. So when the M2 factory function executes it fails to find M1.f1().

kitsonk added a commit to kitsonk/deno that referenced this issue Feb 11, 2020
Moves to using a minimal System loader for bundles generated by Deno.
TypeScript in 3.8 will be able to output TLA for modules, and the loader
is written to take advantage of that as soon as we update Deno to TS
3.8.

System also allows us to support `import.meta` and provide more ESM
aligned assignment of exports, as well as there is better handling of
circular imports.

The loader is also very terse versus to try to save overhead.

Also, fixed an issue where abstract classes were not being rexported.

Fixes denoland#2553
Fixes denoland#3559
Fixes denoland#3751
Fixes denoland#3825
Refs denoland#3301
kitsonk added a commit to kitsonk/deno that referenced this issue Feb 11, 2020
Moves to using a minimal System loader for bundles generated by Deno.
TypeScript in 3.8 will be able to output TLA for modules, and the loader
is written to take advantage of that as soon as we update Deno to TS
3.8.

System also allows us to support `import.meta` and provide more ESM
aligned assignment of exports, as well as there is better handling of
circular imports.

The loader is also very terse versus to try to save overhead.

Also, fixed an issue where abstract classes were not being rexported.

Fixes denoland#2553
Fixes denoland#3559
Fixes denoland#3751
Fixes denoland#3825
Refs denoland#3301
@ry ry closed this as completed in #3965 Feb 12, 2020
ry pushed a commit that referenced this issue Feb 12, 2020
Moves to using a minimal System loader for bundles generated by Deno.
TypeScript in 3.8 will be able to output TLA for modules, and the loader
is written to take advantage of that as soon as we update Deno to TS
3.8.

System also allows us to support `import.meta` and provide more ESM
aligned assignment of exports, as well as there is better handling of
circular imports.

The loader is also very terse versus to try to save overhead.

Also, fixed an issue where abstract classes were not being re-exported.

Fixes #2553
Fixes #3559
Fixes #3751
Fixes #3825
Refs #3301
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 a pull request may close this issue.

2 participants