Skip to content
This repository has been archived by the owner on Sep 16, 2021. It is now read-only.

Missing symbols with ts-protoc-gen #30

Closed
pcj opened this issue Sep 7, 2017 · 8 comments
Closed

Missing symbols with ts-protoc-gen #30

pcj opened this issue Sep 7, 2017 · 8 comments

Comments

@pcj
Copy link
Member

pcj commented Sep 7, 2017

I'm working on getting ts-protoc-gen to compile with rules_typescript. First real project I've used with rules_typescript so it may be is likely something stupid I'm doing.

pcj:~/github/improbable-eng/ts-protoc-gen*bazel$ bazel build //src:src
ERROR: /Users/pcj/github/improbable-eng/ts-protoc-gen/src/BUILD:4:1: Compiling TypeScript (devmode) //src:src failed (Exit 1)
src/util.ts(48,49): error TS2304: Cannot find name 'Buffer'.
src/util.ts(49,14): error TS2304: Cannot find name 'Buffer'.
src/util.ts(52,17): error TS2304: Cannot find name 'process'.
src/util.ts(57,30): error TS2304: Cannot find name 'Buffer'.
src/util.ts(64,14): error TS2304: Cannot find name 'Buffer'.
src/js_service_index.ts(13,26): error TS2304: Cannot find name 'Buffer'.
src/js_service_index.ts(39,5): error TS2304: Cannot find name 'process'.
src/js_service_index.ts(39,30): error TS2304: Cannot find name 'Buffer'.
src/js_service_index.ts(42,5): error TS2304: Cannot find name 'process'.
src/ts_index.ts(14,26): error TS2304: Cannot find name 'Buffer'.
src/ts_index.ts(50,5): error TS2304: Cannot find name 'process'.
src/ts_index.ts(50,30): error TS2304: Cannot find name 'Buffer'.
src/ts_index.ts(53,5): error TS2304: Cannot find name 'process'.

Which made me think the ambient node types are missing, but they seem to be present:

pcj:~/github/improbable-eng/ts-protoc-gen*bazel$ bazel query //:* --output=label_kind | grep '@types/node'
source file //:node_modules/@types/node/package.json
source file //:node_modules/@types/node/index.d.ts
source file //:node_modules/@bazel/typescript/node_modules/@types/node/package.json
source file //:node_modules/@bazel/typescript/node_modules/@types/node/index.d.ts

Any pointers on how to approach it?

improbable-eng/ts-protoc-gen#24

@pcj
Copy link
Member Author

pcj commented Sep 8, 2017

Well, I was able to workaround the above issue by placing

/// <reference types="node" />

In src/util.ts. Not clear on why this needs to be present, seems like it should automagically be picked up in @types/node.

@pcj
Copy link
Member Author

pcj commented Sep 8, 2017

Next issue is a disconnect between expected and actual location of generated declaration files as influenced by the declarationDir (as provided to the compiler in tsc_wrapped.ts):

Printing the expected outputs in _compile_action:

expected output: <bazel-out/local-fastbuild/bin/src/util.js>
expected output: <bazel-out/local-fastbuild/bin/src/util.d.ts>
# ... and so on for the generated js file and generated declaration file pairs

Printing out the compilerOptions object in tsc_wrapped.ts yields:

compiler options: {
  "alwaysStrict": true,
  "sourceMap": false,
  "declaration": true,
  "declarationDir": "/private/var/tmp/_bazel_pcj/4418fe7b2dbf857e1778404007ae0f68/bazel-sandbox/2915656605711346695/execroot/__main__/lib",
  "target": 1,
  "removeComments": true,
  "noImplicitReturns": true,
  "noImplicitAny": true,
  "noUnusedLocals": true,
  "noUnusedParameters": true,
  "strictNullChecks": true,
  "stripInternal": true,
  "noFallthroughCasesInSwitch": true,
  "outDir": "/private/var/tmp/_bazel_pcj/4418fe7b2dbf857e1778404007ae0f68/bazel-sandbox/2915656605711346695/execroot/__main__/bazel-out/local-fastbuild/bin",
  "noEmitOnError": false,
  "typeRoots": [
    "/private/var/tmp/_bazel_pcj/4418fe7b2dbf857e1778404007ae0f68/bazel-sandbox/2915656605711346695/execroot/__main__/node_modules/@types"
  ],
  "downlevelIteration": true,
  "skipDefaultLibCheck": true,
  "module": 1,
  "moduleResolution": 2,
  "rootDir": "/private/var/tmp/_bazel_pcj/4418fe7b2dbf857e1778404007ae0f68/bazel-sandbox/2915656605711346695/execroot/__main__",
  "rootDirs": [
    "/private/var/tmp/_bazel_pcj/4418fe7b2dbf857e1778404007ae0f68/bazel-sandbox/2915656605711346695/execroot/__main__/bazel-out/local-fastbuild/genfiles",
    "/private/var/tmp/_bazel_pcj/4418fe7b2dbf857e1778404007ae0f68/bazel-sandbox/2915656605711346695/execroot/__main__/bazel-out/local-fastbuild/bin",
    "/private/var/tmp/_bazel_pcj/4418fe7b2dbf857e1778404007ae0f68/bazel-sandbox/2915656605711346695/execroot/__main__"
  ],
  "baseUrl": "/private/var/tmp/_bazel_pcj/4418fe7b2dbf857e1778404007ae0f68/bazel-sandbox/2915656605711346695/execroot/__main__",
  "paths": {
    "*": [
      "./*",
      "bazel-out/local-fastbuild/genfiles/*",
      "bazel-out/local-fastbuild/bin/*"
    ]
  },
  "preserveConstEnums": false,
  "experimentalDecorators": true,
  "emitDecoratorMetadata": true,
  "jsx": 2,
  "jsxFactory": "React.createElement",
  "inlineSourceMap": true,
  "inlineSources": true
}

The lib/ part seems to be the issue. If I manually delete the declarationDir via:

delete options["declarationDir"]

All outputs are generated as expected :)

Will look into how declarationDir is computed but wanted to put my thoughts down before I forget...

@pcj
Copy link
Member Author

pcj commented Sep 8, 2017

So it looks like rules_typescript is blissfully unaware of the declarationDir option, but it's in the tsconfig.json for this particular project (ts-protoc-gen).

Probably this needs to be checked in _outputs function, but that may imply needing a new attribute to ts_library to express it? I don't think it's possible to read the tsconfig.json file (?)

@fortuna @alexeagle thoughts?

Reference:

microsoft/TypeScript#6723

microsoft/TypeScript#7170

https://github.com/Microsoft/TypeScript/blob/2e027789606a7b5bcd015cf1173823ed8f83023b/src/compiler/program.ts#L1983

@alexeagle
Copy link
Contributor

Hey @pcj great to see you here! You're among the first to try integrating with this rule, I would not be surprised if you find a bunch more issues, and I'm grateful for all of them.

I have not heard of declarationDir until just now. The theory under bazel is that paths for inputs and outputs are controlled by the bazel TS compiler, not by the user. So my immediate reaction is to disallow or overwrite the declarationDir setting in your tsconfig.json.

What's the use case that drives you to want .d.ts outputs written to a different location from js outputs?

@pcj
Copy link
Member Author

pcj commented Sep 9, 2017

Honestly I agree with you, I don't see the real utility of declarationDir myself, but clearly it exists in the wild.

I'd favor overwriting it / eliminating it in tsc_wrapper.ts and updating the README as I can't imagine anyone would care where files end up in bazel-out/....

Happy to post a PR if you like.

@alexeagle
Copy link
Contributor

My guess is that declarationDir is useful in a packaging step, where you don't want to "clutter" the .js files in your distribution by adding typings in the same place.
We do this for our own project, angular/tsickle, which will migrate to bazel sometime soonish. So we'll either have to change the distribution, or make it possible to break up the files.

That said, if we do support such a thing, I think it belongs in the packaging rule, not in ts_library, because it's currently convenient that the output paths are predictable without knowing anything about the configuration.

Sure, you can do a PR if you're interested. Also, are you thinking of using Bazel for a project?

@alexeagle
Copy link
Contributor

Is this fixed now?

@pcj
Copy link
Member Author

pcj commented Sep 14, 2017

Yep, with workaround noted in #31.

@pcj pcj closed this as completed Sep 14, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants