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

refactor(cli): migrate runtime compile/bundle to new infrastructure #8192

Merged
merged 7 commits into from
Nov 2, 2020

Conversation

kitsonk
Copy link
Contributor

@kitsonk kitsonk commented Oct 30, 2020

Fixes #8060

Couple notes:

  • There will be cleanup post this, specifically in the snapshot creation of the compiler and some refactoring in the 99_compiler.js.
  • There are a couple changes in behaviour.
    • Even when sources are provided, the resulting source keys are the text version of module specifiers. So for example if you send in sources of /foo.ts it will return with file:///foo.ts.js and file:///foo.ts.js.map. This makes it a lot easier to deal with everything internally. Users can provide sources with any URI that was as well. It should also make it easier to feed externally gathered sources to the APIs.
    • The compiler option of types was strange to begin with for the compiler APIs and I think we need revisit supporting that all together anyways.

@ry
Copy link
Member

ry commented Oct 30, 2020

glad to see cli/module_graph.rs go! can cli/tsc.rs be removed too?

@kitsonk
Copy link
Contributor Author

kitsonk commented Oct 30, 2020

Oops. cli/tsc.rs should have been removed too. I messed up during a rebase.

cli/module_graph2.rs Outdated Show resolved Hide resolved
cli/op_fetch_asset.rs Show resolved Hide resolved
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

Great work @kitsonk 🚀 I have only a handful of low-impact comments. Can't wait to land this one!

core/module_specifier.rs Outdated Show resolved Hide resolved
cli/tsc_config.rs Outdated Show resolved Hide resolved
cli/tests/lib_runtime_api.ts Show resolved Hide resolved
cli/program_state.rs Show resolved Hide resolved
cli/module_loader.rs Show resolved Hide resolved
cli/module_graph2.rs Outdated Show resolved Hide resolved
cli/module_graph2.rs Show resolved Hide resolved
cli/module_graph2.rs Show resolved Hide resolved
cli/module_graph2.rs Outdated Show resolved Hide resolved
cli/module_graph2.rs Show resolved Hide resolved
Copy link
Member

@bartlomieju bartlomieju left a comment

Choose a reason for hiding this comment

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

LGTM

@kitsonk kitsonk force-pushed the feat_runtime_compile_bundle branch from c2e4f47 to 16a6dfb Compare November 1, 2020 23:51
@kitsonk kitsonk merged commit fdcc785 into denoland:master Nov 2, 2020
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.

Migrate Deno.compile/Deno.bundle to new tsc/module_graph
4 participants