Skip to content

Hello world example in Typescript #112

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

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

namesty
Copy link

@namesty namesty commented Sep 13, 2023

No description provided.

@namesty namesty requested a review from krisbitney September 13, 2023 17:18
Copy link
Contributor

@krisbitney krisbitney left a comment

Choose a reason for hiding this comment

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

This is very impressive and cool to see!!

Unfortunately, I wasn't able to build it. It looks like it depends on pwr CLI, but it isn't available in the Docker image.

#11 [7/7] RUN /bin/bash -i -c "pwr js build -f ./wrap.js -o build"
#11 0.063 bash: cannot set terminal process group (1): Inappropriate ioctl for device
#11 0.063 bash: no job control in this shell

#11 0.066 bash: pwr: command not found
#11 ERROR: process "/bin/sh -c /bin/bash -i -c \"pwr js build -f ./wrap.js -o build\"" did not complete successfully: exit code: 127
------
 > [7/7] RUN /bin/bash -i -c "pwr js build -f ./wrap.js -o build":
0.063 bash: cannot set terminal process group (1): Inappropriate ioctl for device
0.063 bash: no job control in this shell
0.066 bash: pwr: command not found
------

Dockerfile:14
--------------------
  12 |     COPY ./bundled/wrap.js ./wrap.js
  13 |     # Use pwr to build the script
  14 | >>> RUN /bin/bash -i -c "pwr js build -f ./wrap.js -o build"
--------------------
ERROR: failed to solve: process "/bin/sh -c /bin/bash -i -c \"pwr js build -f ./wrap.js -o build\"" did not complete successfully: exit code: 127

"scripts": {
"bundle": "rollup -c",
"codegen": "polywrap codegen --verbose",
"build": "yarn bundle && npx polywrap build --no-codegen --strategy=image",
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's add yarn codegen to the yarn build script?

yarn codegen && yarn bundle && npx polywrap build --no-codegen --strategy=image

@@ -0,0 +1 @@
# TS Wrap Template
Copy link
Contributor

Choose a reason for hiding this comment

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

Since TS wraps work a bit differently, it would be nice to get some brief explanation of the nuances here (or a link to docs).

const textEncoderShims = require("./textEncoder");
const msgpackShims = require("./msgpack");

const packagesShim = `
Copy link
Contributor

Choose a reason for hiding this comment

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

For jetbrains IDEs, it's helpful to annotate strings that contain code with a comment indicating the language:

// language=javascript
const packagesShim = `
...

This provides syntax highlighting etc.

If it doesn't hurt anything for vs code users, could we add the comment here?

}

//HACK: remove this once codegen is fixed
const __old_subinvoke = __wrap_subinvoke;
Copy link
Contributor

Choose a reason for hiding this comment

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

JavaScript is a wild language 😂

});
}

const fs = {
Copy link
Contributor

Choose a reason for hiding this comment

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

A bit out of scope of this PR, I just want to suggest that we eventually hide everything but the shim declarations from users. My thought is that they should only need to worry about declaring shims, and not about the other content in this file.

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.

2 participants