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

feat: bundle installed script #5276

Merged
merged 3 commits into from
Sep 5, 2020

Conversation

kt3k
Copy link
Member

@kt3k kt3k commented May 13, 2020

This partially addresses #5069. This PR bundles installed script and saved it next to the executable shell/batch script.

The command works like the below:

$ ./target/debug/deno install --allow-net https://deno.land/std/examples/echo_server.ts
Bundling https://deno.land/std/examples/echo_server.ts
Download https://deno.land/std/examples/echo_server.ts
Emitting bundle to "/Users/kt3k/.deno/bin/echo_server.js"
2661 bytes emmited.
✅ Successfully installed echo_server
/Users/kt3k/.deno/bin/echo_server

I checked 2 examples working on mac and windows:

./target/debug/deno install --allow-read https://deno.land/std/examples/cat.ts
./target/debug/deno install --allow-net https://deno.land/std/examples/echo_server.ts
  • check examples on windows.

Closes #5069

@kt3k kt3k force-pushed the feature/bundle-install branch from d686a68 to 773651a Compare May 13, 2020 18:20
@bartlomieju bartlomieju added cli related to cli/ dir feat new feature (which has been agreed to/accepted) labels May 15, 2020
@kt3k kt3k force-pushed the feature/bundle-install branch 2 times, most recently from e092a49 to 9c0617a Compare May 25, 2020 18:16
@kt3k
Copy link
Member Author

kt3k commented May 25, 2020

I checked the the command works on a windows machine as well.

@ry @bartlomieju PTAL

@bartlomieju
Copy link
Member

Thanks @kt3k, can you fix the CI? I tried merging master but it seems it requires more changes. We'll try to land it for v1.1

@kt3k kt3k force-pushed the feature/bundle-install branch from a3b86fb to 8ea9dab Compare June 8, 2020 16:49
@kt3k
Copy link
Member Author

kt3k commented Jun 8, 2020

@bartlomieju Thank you for the ping! I'm trying.

@kt3k
Copy link
Member Author

kt3k commented Jun 8, 2020

done!

@bartlomieju
Copy link
Member

I tested locally and it works fine. Unfortunately #4207 is still not resolved meaning that any script installed using TLA will break now.

@kitsonk
Copy link
Contributor

kitsonk commented Jun 8, 2020

any script installed using TLA

Only top level for await... TLA works fine.

@CLAassistant
Copy link

CLAassistant commented Jul 27, 2020

CLA assistant check
All committers have signed the CLA.

@ry
Copy link
Member

ry commented Aug 18, 2020

@kt3k can you rebase this? I would be interested in landing it now.

@kt3k
Copy link
Member Author

kt3k commented Aug 19, 2020

@ry OK. I'll try today and tomorrow.

@kt3k kt3k force-pushed the feature/bundle-install branch 3 times, most recently from 1dec353 to 758596c Compare August 19, 2020 14:53
@kt3k kt3k force-pushed the feature/bundle-install branch from 758596c to b73e7d5 Compare August 19, 2020 15:52
@kt3k
Copy link
Member Author

kt3k commented Aug 19, 2020

Rebased!

By the way I dropped the copying of tsconfig into bin directory because this bundles the script into .js and tsconfig.json is no longer necessary for running the bundled script.

@ry ry added this to the 1.4.0 milestone Aug 20, 2020
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, thank you @kt3k and sorry it took so long to land!

@bartlomieju bartlomieju merged commit 34e98fa into denoland:master Sep 5, 2020
@kt3k kt3k deleted the feature/bundle-install branch September 5, 2020 16:47
piscisaureus added a commit that referenced this pull request Sep 16, 2020
piscisaureus added a commit to piscisaureus/deno that referenced this pull request Sep 16, 2020
This reverts the changes introduced by PR denoland#5276, which made
`deno install «script»` automatically bundle the script's dependencies.
It broke the `deno install` command for a large number of scripts.

This reverts commit 34e98fa.
piscisaureus added a commit to piscisaureus/deno that referenced this pull request Sep 16, 2020
This reverts the changes introduced by PR denoland#5276, which made
`deno install «script»` automatically bundle the script's dependencies.
It broke the `deno install` command for a large number of scripts.

This reverts commit 34e98fa.

Closes: denoland#7492
piscisaureus added a commit to piscisaureus/deno that referenced this pull request Sep 16, 2020
This reverts the changes introduced by PR denoland#5276, which made
`deno install «script»` automatically bundle the script's dependencies.
It broke the `deno install` command for a large number of scripts.

This reverts commit 34e98fa.

Closes: denoland#7492
vitormmatos pushed a commit to vitormmatos/deno that referenced this pull request Sep 21, 2020
This reverts the changes introduced by PR denoland#5276, which made
`deno install «script»` automatically bundle the script's dependencies.
It broke the `deno install` command for a large number of scripts.

This reverts commit 34e98fa.

Closes: denoland#7492
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cli related to cli/ dir feat new feature (which has been agreed to/accepted)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

deno install should bundle before installation
5 participants