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

[storage][sample]fix sample debug in launch.json #7030

Closed
wants to merge 1 commit into from

Conversation

ljian3377
Copy link
Member

The debug config for "Debug Typescript Samples" is broken by PR #6496.
azure-sdk-for-js/sdk/storage/storage-blob/.vscode/launch.json

    {
      "type": "node",
      "request": "launch",
      "name": "Debug Typescript Samples",
      "program": "${workspaceFolder}/samples/typescript/basic.ts",
      "preLaunchTask": "npm: build:ts-samples",
      "outFiles": ["${workspaceFolder}/dist-esm/samples/typescript/*.js"]
    },
  1. The generated JavaScript files are now under the ${workspaceFolder}/samples/typescript/dist/samples/typescript/src/ directory.
  2. The npm build:ts-samples command is removed.

I haven't figured out the perfect fix for the preLaunchTask since build:prep-samples will delete the main function of each individual sample file.

@HarshaNalluru
Copy link
Member

@willmtemple
My suggestion for the preLaunchTask would be to do

  1. npm pack and get the tar zip of the package
  2. install the zipped package in the samples folder

rather than relying on prep-samples to change the sample files every time.

@willmtemple
Copy link
Contributor

Currently we rely on the package sdk/service/package/node_modules folder to handle all dependency resolution, so we'll need to install deps from samples/javascript/package.json, then compile and pack all the dependencies of the samples and of the package itself that are in the repo, and then install them individually if we go the pack route. I think we should move away from prep-samples but pack is also seeming to me like it will have some problems.

The reason the call to main() is removed in the processed files is because the run-samples script uses require() to load the samples instead of running them as shell commands. The vast majority of the samples aren't set up to exit with a non-zero exit code in case of failure (since they all catch rejections after their main() calls), so using require() was a way to detect whether or not a sample failed and to aggregate sample failures into a short message at the end of execution without having to add another line to every sample.

As a quick fix, there should be a way to invoke node with a little shim program that imports the sample we want and runs its main function, and then we should be able to debug that. Something like this:

node -e "require('./samples/typescript/dist/samples/typescript/src/basic').main()"

I'll spend some time looking in to this.

@ramya-rao-a
Copy link
Contributor

@willmtemple Are there any updates on this? If not, can we create an issue to track the required work?

@ljian3377
Copy link
Member Author

ljian3377 commented Nov 2, 2020

Close as our debug conf is changed in #10145.

@ljian3377 ljian3377 closed this 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.

4 participants