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

Support named exports on bundles. #3352

Merged
merged 7 commits into from Nov 20, 2019

Conversation

@kitsonk
Copy link
Contributor

kitsonk commented Nov 15, 2019

Fixes #3283

This PR re-exports the exports of the main module passed on the command line. So if your main module looked something like this:

export { foo } from "./foo.ts";
export const bar = "bar";
export default "baz";

And you generated it into a bundle named myLib.bundle.js you could consume it like this:

import myLib, { foo, bar } from "./myLib.bundle.js";

There was a partial solution tabled in #3283 to provide all the exports of the main module as the default export for the bundle. Looking into how to determine what is exported out of the main module, it was actually fairly easy to just generate the named export code.

@kitsonk kitsonk force-pushed the kitsonk:bundle_exports branch 2 times, most recently from 8daadd7 to 54bc5cc Nov 15, 2019
@@ -0,0 +1,178 @@
// Copyright 2018-2019 the Deno authors. All rights reserved. MIT license.

This comment has been minimized.

Copy link
@ry

ry Nov 15, 2019

Collaborator

This file is the output of deno bundle command? I think it would be good to automatically generate it during the tests.

This comment has been minimized.

Copy link
@kitsonk

kitsonk Nov 15, 2019

Author Contributor

👍

This comment has been minimized.

Copy link
@kitsonk

kitsonk Nov 16, 2019

Author Contributor

The only problem is that we don't support a pre-req in our integration tests, and I can't be guaranteed execution order or dependency. So to get one integration test to output it, and one integration test to consume it seems fragile, right?

This comment has been minimized.

Copy link
@hayd

hayd Nov 16, 2019

Contributor

If you run and consume in a single test it wouldn't be fragile.

This comment has been minimized.

Copy link
@kitsonk

kitsonk Nov 16, 2019

Author Contributor

Yes, but our integration tests don't support two invocations of Deno.

This comment has been minimized.

Copy link
@ry

ry Nov 16, 2019

Collaborator

You’d need to write a custom one. See for example the js_unit_tests https://github.com/denoland/deno/blob/master/cli/tests/integration_tests.rs#L41

@ry

This comment has been minimized.

Copy link
Collaborator

ry commented Nov 18, 2019

This patch is awesome. I tried it with std/http/server.ts and it worked.

I did get a failure when trying it with std/encoding/yaml.ts. I think this is because yaml.ts doesn't export any symbols directly, but rather via another module. Do you have any ideas about how this could be supported? I have the same problem with the documentation generator on the website...

@kitsonk

This comment has been minimized.

Copy link
Contributor Author

kitsonk commented Nov 18, 2019

Cool, need a test case. I will have a look and see what is wrong with that.

kitsonk added 3 commits Nov 15, 2019
@kitsonk

This comment has been minimized.

Copy link
Contributor Author

kitsonk commented Nov 18, 2019

Ok, it appears that it does export a symbol named __export, of which it has runtime code which determines what exports gets named. So, what I need to figure out is if that symbol is walkable to to the other SourceFiles, to get their symbols to determine what their names are, etc... until we have all the exports determined.

@kitsonk kitsonk force-pushed the kitsonk:bundle_exports branch from 677a91c to 68af62f Nov 19, 2019
@kitsonk

This comment has been minimized.

Copy link
Contributor Author

kitsonk commented Nov 19, 2019

Ok, I found a better compiler API that will give you all the exported members names, even if they are re-exports of another module. The one downside is that it also picks up "type only" exports as well. They should just result in undefined values in the resulting module, but I don't fully like that because some people looking at the bundle would think there is something there (including someone trying to consume the bundle in an intelligent editor). I have put some logic in there to try to weed out things that aren't "real", but I have also opened microsoft/TypeScript#35183 to see if there is some cleaner way.

ry added 2 commits Nov 20, 2019
// check the output of the test.ts program.
assert_eq!(output.stdout, b"Hello\n");
assert_eq!(output.stderr, b"");
}

This comment has been minimized.

Copy link
@ry

ry Nov 20, 2019

Collaborator

@kitsonk I've modified the test - I think this is functionally equivalent to what you had before. PTAL.

This comment has been minimized.

Copy link
@kitsonk

kitsonk Nov 20, 2019

Author Contributor

Ha! I was working on it right now... just haven't had as much as I would like, but yeah, this looks like what I was doing. Thank you.

Copy link
Contributor Author

kitsonk left a comment

LGTM

// check the output of the test.ts program.
assert_eq!(output.stdout, b"Hello\n");
assert_eq!(output.stderr, b"");
}

This comment has been minimized.

Copy link
@kitsonk

kitsonk Nov 20, 2019

Author Contributor

Ha! I was working on it right now... just haven't had as much as I would like, but yeah, this looks like what I was doing. Thank you.

@ry
ry approved these changes Nov 20, 2019
Copy link
Collaborator

ry left a comment

LGTM

@ry ry merged commit 8d977d0 into denoland:master Nov 20, 2019
7 checks passed
7 checks passed
test macOS-latest
Details
test windows-2019
Details
test 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
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.