Bundling V2

[kitsonk]

Refs #2357
Refs #2467

The initial implementation of bundling is an "MVP". There are a couple things we might want to consider in a future version of the bundling:

• The bundle is loaded via just in the runtime, using Deno.core.evalContext(). It is worth investigating if this is the optimal way. At the very least, causes the stack traces to be a bit of a challenge, because a source name isn't provided:

  $ deno run -A ../deno_std/bundle/run.ts error_test.js
  <unknown>:37:14
          throw Error("exception from mod1");
                ^
  Uncaught Error: exception from mod1
      at throwsError (<unknown>:37:15)
      at foo (<unknown>:45:19)
      at <unknown>:47:5
      at instantiate (file:///Users/kkelly/github/deno_std/bundle/utils.ts:88:14)
      at main (file:///Users/kkelly/github/deno_std/bundle/run.ts:8:3)
  

  This should be just a change to the .evalContext() API to allow a source name to be supplied in addition to the code, but this would lead to the next problem:

• Code points are not being remapped in the errors. The source map is inlined in the bundle in base64. In order to get the error remapping to work, we would need an Op to send the source map back to Rust to be used when remapping error stacks.

• Consider building the loader into the binary and add a flag like -b to indicate that the main file should be loaded as a bundle. The deno_std loader requires the usage of --allow-read but having it build in and behind a flag would mean that Rust could load the bundle, and use libdeno directly to inject the loader and eval the bundle.

[kitsonk]

• Consider tree shaking/dead code elimination while bundling. This is rather a large thing though, so maybe v3? It requires some complex analysis of the code and most of the bundlers that do tree shaking or dead code elimination utilise some form of an ES tree AST instead of a TypeScript AST. That would mean embedding another parser into Deno just to do the analysis, which just seems not very logical. Even then, tree-shaking is complex and error prone (like thinking something doesn't have side effects but does).

[hayd]

A minor issue: Deno.args doesn't work as expected (it uses unmodified args rather than what's passed to the bundle). The example I had was bundling fmt.

[kitsonk]

@hayd I am not sure I understand that issue, can you be more specific.

[hayd]

@kitsonk Example:

 % deno bundle https://deno.land/std/prettier/main.ts
[10/10] Downloading https://raw.githubusercontent.com/denoland/deno_std/master/prettier/vendor/parser_markdown.jsBundling "main.bundle.js"
Emitting bundle to "main.bundle.js"
12.4 MB emitted.

 % deno -A https://deno.land/std/bundle/run.ts main.bundle.js x.ts
strictEqual failed. actual = 3 expected = 2
https://raw.githubusercontent.com/denoland/deno_std/master/testing/asserts.ts:118:14
        throw new AssertionError(msg);
              ^
Uncaught AssertionError: Expected exactly two arguments.
    at AssertionError (https://raw.githubusercontent.com/denoland/deno_std/master/testing/asserts.ts:11:5)
    at assertStrictEq (https://raw.githubusercontent.com/denoland/deno_std/master/testing/asserts.ts:150:11)
    at load (https://raw.githubusercontent.com/denoland/deno_std/master/bundle/utils.ts:101:3)
    at main (https://raw.githubusercontent.com/denoland/deno_std/master/bundle/run.ts:6:22)
    at https://raw.githubusercontent.com/denoland/deno_std/master/bundle/run.ts:11:1

[kitsonk]

Ok, I thought it might be that. We should open an issue on the loader in std about dealing with shifting off the first the arg somehow. I don't think it would be an issue for the built in loader because any args would be consumed before they hit the runtime.

[kitsonk]

While I get the "worry" about AMD, hopefully people would look beyond that and not need to introspect the format of a bundle.

Bundling (merge/concat imports, tree shake, targeting, transforming, and uglify-ing) needs to be supported. But either choose a tool - like rollup as you chose prettier for formatting - or roll your own that supports a configuring all of the standard options modern projects require. Like: bundle type, naming, transformations, hooks, ...

I am not sure these would need to be supported. Most of those features are meaningful in a situation where you are serving up content to a browser. Creating browser consumable assets is not the intent of bundle, nor should people focus on that feature ever delivering it. If people need that feature, they should use something that is fit for purpose. The purpose and intent for bundling in Deno is to create a standalone, single artefact with no remote dependencies, nothing more. This is with an eventual eye towards generating stand alone executables.

Essentially, the default leaves it as an ES module that is collapsed to a single file.

That is the problem, there is no such way. There is no way to concatenate ES modules. It requires fairly complex static analysis of the modules to bundle and rewriting each module. Even then there are some challenges with how to handle things like default exports from modules (because then those can't be consumed outside the bundle easily). AMD is the path of least resistance, as the TypeScript compiler supports its output and the modules are easily concatenate-able without having to re-parse them and statically analyse them and rewrite them.

[kitsonk]

If the purpose of bundle is to create a deployable package (probably) without dependencies, then it must allow development teams the opportunity to assemble that package based on the requirements of the target environment.

The purpose of deno bundle is to bundle code for deno, not other platforms. So it meets requirements of the target environment.

Please spend some time reviewing rollup, as it is not only the industry standard for library bundling, but it very nicely resolves precisely the problem you claim cannot be done.

Please understand I am the one who originally implemented rollup for creating the internal bundles of JavaScript code for Deno. I have also contributed to the rollup project. I know its architecture really well, and have worked really closely with bundlers for many years. I am not saying they aren't solvable, I am saying a) ES Modules are not concatenatable b) to have something that works is complex and that is a load of complexity for no value for what the feature is intended for.

I think it is extremely important that Dino not put any requirements or preference on the input/output code.

I disagree Deno needs to be opinionated about the code it runs. deno bundle is to create bundles of code to run in Deno. Creating (or validating that an existing bundler runs under Deno) is a seperate concern than Deno creating bundles for its own consumption.

I think you are confusing features to make Deno more usable with workload you might want to run under Deno.

[tunnckoCore]

The purpose of deno bundle is to bundle code for deno, not other platforms. So it meets requirements of the target environment.

This doesn't make any sense to me, sorry 🤔 Why would one need a bundle that does nothing, practically useful except just similar thing to concatenation and types stripping? They can run js/ts files anyway. Why they need a js bundle with AMD, which... ahh.

Including Rollup with some default config would be the best way. This will open the way to producing 3 output targets, and may help in compat with Node and browsers, and why not easier transition to Deno in general.

Everyone coming to Deno will think of this command in that way anyway, because they see that term all the time and know it what it does for them to now, for example in Nodejs land.

Both Nodejs and Deno are tools for helping to develop both the Web and the needed things for developers developing the Web, eg. CLIs, parsers, compilers, utils.

[kitsonk]

This doesn't make any sense to me, sorry 🤔 Why would one need a bundle that does nothing, practically useful except just similar thing to concatenation and types stripping? They can run js/ts files anyway. Why they need a js bundle with AMD, which... ahh.

So you have a single artefact for your programme, which makes distributing a self-contained Deno workload easier. It is part of the path to having a single binary distributable.

Including Rollup with some default config would be the best way.

Again, running rollup under Deno is not this feature. That is something that can/could be solved in the wider community.

Everyone coming to Deno will think of this command in that way anyway

Naming things is hard and everyone is a highly subjective term. This is what documentation is for.

[tunnckoCore]

It is part of the path to having a single binary distributable.

Which will be used where? Only on the server, only possible to run from Deno?

everyone is a highly subjective term

Okay, mostly everyone. Most may refer to bundling with these assumptions.

[kitsonk]

Which will be used where? Only on the server, only possible to run from Deno?

See #986

[ghost]

I tried to leave this alone, but I just can't. This is IMO just a really really bad idea. I'm going to make one final plea and then leave you alone.

Dino is two things: a development tool (like go-cli) and a target environment. Clearly, the inclusion of lint, format, test, ... put build targets right smack in the middle of the scope of a Dino development tool. Dino the development tool should support multiple target environments where Dino the target environment is just one of them.

[ry]

It seems some of the comments have been deleted in this thread, so I'm missing some context.

@kitsonk wrote:

Creating browser consumable assets is not the intent of bundle, nor should people focus on that feature ever delivering it.

Why not? We're using deno bundle already for the website. Since deno is trying to be explicitly browser compatible, there's no reason to limit the scope of the bundle feature to not include websites. I consider issues like #2553 to be fairly important API design problems.

@Fedler

This is IMO just a really really bad idea

Can you be more explicit in stating what you think is a bad idea?

[kitsonk]

Why not? We're using deno bundle already for the website. Since deno is trying to be explicitly browser compatible, there's no reason to limit the scope of the bundle feature to not include websites. I consider issues like #2553 to be fairly important API design problems.

@ry the deleted comments were suggesting that if we are trying to do that that we should produce an infinitely configurable bundling feature, specifically one that doesn't use AMD. We shouldn't break browser compatibility with this feature, but we shouldn't specifically be concerned about optimising for browser compatibility. People we assuming, incorrectly, that bundle was a general purpose bundler that could replace things like webpack, rollup, etc. It is not, nor should people should expect it to be.

Dino is two things: a development tool (like go-cli) and a target environment. Clearly, the inclusion of lint, format, test, ... put build targets right smack in the middle of the scope of a Dino development tool. Dino the development tool should support multiple target environments where Dino the target environment is just one of them.

The lint, format, test are opinionated features, just like bundle. There are designed to make Deno workloads easier, not a swiss army knife of features for building code for browsers. People can utilise workloads on top of Deno that do that.

[ry]

specifically one that doesn't use AMD

Yeah, we can't do that.. I'm not even sure what it means to bundle in some form other than AMD or System modules.

We shouldn't break browser compatibility with this feature, but we shouldn't specifically be concerned about optimising for browser compatibility. People we assuming, incorrectly, that bundle was a general purpose bundler that could replace things like webpack, rollup, etc. It is not, nor should people should expect it to be.

The complexity of webpack and rollup is largely due to the complexity of the node module system and the need to apply transpilation steps. We have neither of those, so we're able to provide a general purpose bundler for the web and deno cli programs without much trouble. Or maybe I don't understand what "general purpose bundler" implies.

I think the one feature we're missing is tree shaking ... but that sounds hard and also like something that TS should be providing.

[kitsonk]

Yeah, we can't do that.. I'm not even sure what it means to bundle in some form other than AMD or System modules.

Yes, that was my point earlier in the thread to address some of the "why isn't it an ESM module or an IIFE" indicating that rewriting this way is actually non-trivial and that people shouldn't be concerned about the format of the bundle and wasn't specifically designed to be an optimised build tool for all browser workloads.

Or maybe I don't understand what "general purpose bundler" implies.

I think the argument was for one that specifically wasn't AMD, but to me "general purpose" would include the ability to add plugins to be able to deal with other static assets, like text, JSX transpilation, CSS, etc. All things you would expect/get from webpack, rollup, etc.

I think the one feature we're missing is tree shaking ... but that sounds hard and also like something that TS should be providing.

Tree shaking is something specifically the TypeScript team have ruled out of scope for them as a project. They have made adjustments to transpilation to support effective tree shaking (like marking code in comments as having no side effects which can't be easily determined by a tree shaker as elidable, when the code itself is ambigious). I tend to agree with it being out of scope. Tree shaking is complex, and there are very specialised tools to do that, doing that well isn't the job of TypeScript. It does two things (provide an erasable type system and provide syntactical transpilation), and we only need it to do one thing, which is enforce an erasable type system.

If tree shaking is a critical core feature of Deno (which I don't think it should be) we should look at incorporating a proper tool that does that, or build something that directly works with the TypeScript AST to do that analysis. That being said, I don't believe there is a tree-shaker (or bundler) that works directly with the TypeScript AST. FuseBox is looking at moving that direction IIRC, but still uses another AST parser to actually do that static analysis.

[kitsonk]

Also, other features of a general purpose bundler would be code splitting and minfication. All things hugely important to building an effective bundle for an in browser experience, all which would not be useful to Deno directly.

[hayd]

It seems like there's three independent environment targeting issues:

• bundle for both browsers and deno (already if don't use Deno?)
• support node code in deno (efforts made already with denolib/node)
• bundle for node or other non-browser platforms (won't fix)

Perhaps I'm missing something... But bundling for node seems redundant/pointless (just use deno). If you want to bundle node code for node, don't use deno!

[kitsonk]

As mentioned in #3196 I will work on getting deno bundle to generate a fully standalone script that is designed to run under Deno, so people don't have to get hung up on the implementation.

[kitsonk]

I've been working on it, and I sort of have everything done, but I have hit a bit of a snag, which I will have to work around. When emitting a "bundle" the TypeScript compiler determines what the internal module specifiers are going to be, and it doesn't match any of the input information about the module. It appears to find the common root for the modules in the program and strips off the extensions when emitting the bundle.

The impact is that to instantiate the "main module" for the bundle in a way that matches how it would be done if you were loading the unbundled version can't be done without "guessing" at what its module specifier in the bundle will be.

I have opened an issue at microsoft/TypeScript#35052 for it, but I doubt it would arrive any time soon, so we will have to "guess" at what the specifier will be in the bundle.

[sholladay]

I know this has already been addressed, so feel free to ignore, but to add my two cents... I also expect deno bundle to output an ES module, at least by default. Rollup is able to do that, so I'm kind of surprised this isn't an easy thing to do with the TS compiler.

I don't think non-standard legacy cruft like AMD belongs in Deno, except maybe as some sort of opt-in or adapter thing from deno's std lib. The whole point, at least to me, is to build the next generation of tooling. If AMD is the only option, I feel like this feature should wait until it's possible to do it properly.

[kitsonk]

It does output an ES Module. A totally valid ES Module.

Rollup does extensive static analysis, and figures out how things can be scoped and named in the final module. When it cannot flatten, it has to come up with some alternative approaches.

Rollup is able to do that, so I'm kind of surprised this isn't an easy thing to do with the TS compiler.

While Rollup is written in TypeScript, it doesn't use the TypeScript compiler to do the static analysis, it uses acorn. When I spoke to Daniel R about it a while back, he indicated that there is no "bundler" that utilises the TypeScript compiler, though FuseBox was investigating it. I think they said to me, that it is aspirational for them. The rules around flattening/merging/re-naming/namespacing/creating closures are really complex. With either AMD or SystemJS singlefile output we get that all for free and the TypeScript team maintain it.

I don't think non-standard legacy cruft like AMD belongs in Deno,

But the non-standard internal innards of Rollup would?

If AMD is the only option, I feel like this feature should wait until it's possible to do it properly.

Define properly. "Not using AMD" is not a good definition. I swear, I could rename the defines in the bundle output to notAMDDefine and people wouldn't have anything to complain about. You are suggesting ripping out a feature that is already landed, that works perfectly well. I need better Jedi skills... "These are not the droids you are looking for..."

[sholladay]

It does output an ES Module. A totally valid ES Module.

Oh, this point was not clear to me. So it's AMD inside of a file with ESM semantics {e.g. implicit strict mode)? That's better, at least.

While Rollup is written in TypeScript, it doesn't use the TypeScript compiler to do the static analysis, it uses acorn.

I know. I merely meant that, conceptually, I would've expected the TS compiler to be more advanced/capable in this respect compared to Rollup (to analyze and transform code), given that it's a compiler for a superset of the language and TypeScript has had to deal with ESM-like import/export for a very long time.

With either AMD or SystemJS singlefile output we get that all for free and the TypeScript team maintain it.

This is the main benefit, as far as I can tell. And it makes perfect sense to me. I just think users won't care. At least not compared to more practical concerns about the size of the bundle, implicit strict mode, top-level await, etc. Maybe all of these things can be fixed/optimized to a point where it's moot, I'm not sure. But they seem like valid reasons to be concerned about the usage of AMD, even if the gut reaction is wrong. One of my first thoughts was, "What does the loader look like, is it something big like RequireJS?" No, of course not, but you can see how there is some baggage associated with it.

But the non-standard internal innards of Rollup would?

As a user, of course I'd prefer the innards of Rollup to be included in Deno than AMD boilerplate to be included in all of my bundles. This isn't to say that people don't care about the size/complexity of Deno itself, just that it may feel less urgent. Also, Deno's innards can be interated on with less impact than changing the bundle output.

Define properly.

Certainly minimal bundle size (boilerplate) is important for this feature, as are TLA support and implicit strict. In terms of the proper implementation, I'm sure we all wish flat ESM output were baked into the TS compiler. But until then, I would've thought one IIFE per module in the bundle would be the way to go. I might be missing something that makes that difficult, though.

[kitsonk]

Oh, this point was not clear to me. So it's AMD inside of a file with ESM semantics {e.g. implicit strict mode)? That's better, at least.

Yes, the current version of the bundling outputs an ESM module and would fail if loaded in the browser as a script (instead of a module) though you just made me realise, that if the root module has no exports, we don't do an export {}; to force the module instead of script. Should do that, so that no one accidentally tries to load it in a browser wrongly.

I would've expected the TS compiler to be more advanced/capable in this respect compared to Rollup

Yeah, it would be great, but the things that the TypeScript compiler is better at aren't any use when trying to flatten a set of modules. You can get a huge amount of information about those modules, and you can generate along side those an AST which you could get TS to emit to ESM, but the whole logic of Rollup is how to transform the source AST in a valid way.

Certainly minimal bundle size (boilerplate) is important for this feature, as are TLA support and implicit strict.

Getting smaller boilerplate is certainly something we could work on. I've kept it a bit verbose at the moment, but it could be reduced quite a bit, and written in a far more efficient way. It isn't even an AMD loader though to be frank, it is a very very minimal loader that wouldn't support a lot of the AMD spec. The lack of TLA support is because TypeScript doesn't support it yet, and we are hacking around it. It is a transitory issue. The implicit strict, yeah, I think I can turn off the explicit "use strict"; that TypeScript is putting in every module definition in the bundle. Because it has to be evaluated as an ES Module, it will always be evaluated in strict mode and the pragma is redundant.

But until then, I would've thought one IIFE per module in the bundle would be the way to go.

Effectively that is what we have. It is just that it isn't immediately invoked (because module resolution and instantiation matters, especially supporting potentially circular references which ESM supports and we support in the bundles). Even with an IIFE you would have to pass in the other dependencies. You can't have "export"/"import" in the IIFE, so you have to write those in some other format, which all of this comes with the AMD or SystemJS format. It just happens that the SystemJS format is longer than the AMD one. One thing we can't get TypeScript to do easily is to not rewrite dynamic import() statements to the other format, so this means we have to create enough of a require() to pass in to modules so that we can then run a dynamic import() on behalf of the module. So that is the most AMD like thing (but even then the implementation in the boilerplate just does that, versus support a full local require()). Basically the boilerplate only supports the way that TypeScript transforms modules to AMD. It can still be made smaller though, which we should do.

[brainkim]

Just had a thought after seeing the top-level await PR merged into typescript. Y’all can do whatever you want with deno, but one really important consideration between choosing AMD vs SystemJS for bundling is that only SystemJS has a story for promises/top-level await.

https://github.com/systemjs/systemjs/blob/master/docs/system-register.md
vs
requirejs/requirejs#1769

[kitsonk]

@brainkim the merge of TLA will fix the issue we are having for bundling without switching formats.

[brainkim]

It seems like you are using the AMD flag for --module?

deno/cli/js/compiler.ts

Line 388 in 658ec2a

module: ts.ModuleKind.AMD,

microsoft/TypeScript#35813

This removes our restriction around using an await expression at the top level of a file when the following conditions are met:

The containing file is an external module (or --isolatedModules is provided)
The --target is >= ScriptTarget.ES2017 (minimum version required for await keyword)
The --module is either esnext or system.

Like I said, y’all know best :)

[kitsonk]

@brainkim sorry, I didn't have my thinking cap on... You are right, we've got a bit of problem. The define factory in AMD technically can't return a promise as part of the module definition. It does mean to support TLA in bundles we will have to look at SystemJS as the output format. Grrr... This is going to get a bit more complicated.

[sholladay]

What actually happens when using --module=esnext? Maybe it's worth trying again with a recent version of TS.

[kitsonk]

@sholladay it does not support output to a single out file. The standard does not support concatenation.

[kitsonk]

Closing in favour of #4549