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

Improvements to bundling. #3965

Merged
merged 6 commits into from Feb 12, 2020
Merged

Improvements to bundling. #3965

merged 6 commits into from Feb 12, 2020

Conversation

@kitsonk
Copy link
Contributor

kitsonk commented 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.

It is worth noting that this cannot be used for the internal snapshot bundles, because it turns the whole instantiation process into an async process which executes out of turn, which causes some problems in setting up the runtime environment. The long term plan though for the snapshots is not to use a "bundle" at all, but instead to inject each module into the isolate.

Fixes #2553
Fixes #3559
Fixes #3751
Fixes #3825
Refs #3301

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 #2553
Fixes #3559
Fixes #3751
Fixes #3825
Refs #3301
@kitsonk kitsonk requested review from bartlomieju and ry Feb 11, 2020
Copy link
Contributor

bartlomieju left a comment

LGTM, I defer to @ry to land

deno_typescript/lib.rs Show resolved Hide resolved
Copy link
Collaborator

ry left a comment

Looks good.

It would be useful to have the test case from #3825 in the integration tests. (Note we already have //cli/tests/circular1.js, maybe that can be reused...)

@ry

This comment has been minimized.

Copy link
Collaborator

ry commented Feb 11, 2020

Oops, I tried removing bundle_loader.js but it's used by mksnapshot_bundle... I wonder if mksnapshot_bundle couldn't also use this new system bundler?

This reverts commit 13ace67.
@kitsonk

This comment has been minimized.

Copy link
Contributor Author

kitsonk commented Feb 11, 2020

@ry no... mentioned in the comments above:

It is worth noting that this cannot be used for the internal snapshot bundles, because it turns the whole instantiation process into an async process which executes out of turn, which causes some problems in setting up the runtime environment. The long term plan though for the snapshots is not to use a "bundle" at all, but instead to inject each module into the isolate.

In particular, the way we deal with Deno.core caused no end to problems when the bundle instantiated modules asynchronously. I know @bartlomieju tried to rework it to just inject each individual module, that would be better, but I know he ran into issues there too. I know it is a small bit of "duplication" but I think it is better to live with it for a period of time, too.

I will add a test case for #3825.

kitsonk added 2 commits Feb 12, 2020
@ry
ry approved these changes Feb 12, 2020
Copy link
Collaborator

ry left a comment

LGTM - thanks kitson!

@ry ry merged commit 6bd846a into denoland:master Feb 12, 2020
7 checks passed
7 checks passed
test_release macOS-latest
Details
test_release windows-2019
Details
test_release ubuntu-16.04
Details
test_debug ubuntu-16.04
Details
bench ubuntu-16.04
Details
lint ubuntu-16.04
Details
license/cla Contributor License Agreement is signed.
Details
linhub15 added a commit to linhub15/deno that referenced this pull request Feb 13, 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 denoland#2553
Fixes denoland#3559
Fixes denoland#3751
Fixes denoland#3825
Refs denoland#3301
geoFlux added a commit to geoFlux/deno that referenced this pull request Feb 14, 2020
* master:
  v0.33.0
  fix: appended CRLF to end of trailer headers (denoland#3989)
  Clean up fmt flags and path handling (denoland#3988)
  Improvements to bundling. (denoland#3965)
  fix: Correctly determine a --cached-only error (denoland#3979)
  chore: share HTTP server between tests (denoland#3966)
  dont use env vars in multiple installer tests (denoland#3967)
  feat(node): add EventEmitter.errorMonitor (denoland#3960)
  fix(file_server): don't crash on "%" pathname (denoland#3953)
  update references to testing/mod.ts in manual (denoland#3973)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
3 participants
You can’t perform that action at this time.