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

Cannot build v0.28.1 #547

Closed
marcusradell opened this issue Mar 12, 2019 · 14 comments
Closed

Cannot build v0.28.1 #547

marcusradell opened this issue Mar 12, 2019 · 14 comments
Assignees
Labels
api: pubsub Issues related to the googleapis/nodejs-pubsub API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.

Comments

@marcusradell
Copy link

  • OS: Ubuntu
  • Node.js version: 10.15.1
  • npm version: 6.4.1
  • @google-cloud/pubsub version: 0.28.1

Steps to reproduce

In a new project:

  • npm install typescript @google-cloud/pubsub
  • Create a file src/index.ts with content:
import { PubSub } from "@google-cloud/pubsub";

console.log("Hello world!");
  • npx tsc --init
  • Set rootDir and outDir in tsconfig.json.
  • run npx tsc and the console output should give you a typescript error:
~/code/labs/pub-sub (master #)$ npx tsc

node_modules/google-gax/build/src/streaming.d.ts:47:42 - error TS2507: Type '{ default: DuplexifyConstructor; obj(writable?: Writable | undefined, readable?: Readable | undefined, streamOptions?: DuplexOptions | undefined): Duplexify; }' is not a constructor function type.

47 export declare class StreamProxy extends Duplexify {
                                            ~~~~~~~~~
Found 1 error.

I tried to downgrade the pubsub version a bit but it didn't solve it.

@marcusradell
Copy link
Author

Seems like v0.26.0 and v0.27.0 works, but only if I add "noImplicitAny": false to tsconfig.

It still fails on v0.28.0 and v0.28.1 with "noImplicitAny": false set.

@callmehiphop callmehiphop added type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns. priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. labels Mar 12, 2019
@JustinBeckwith
Copy link
Contributor

I can reproduce this, and have no earthly idea why our install test doesn't catch it.

@JustinBeckwith
Copy link
Contributor

Actually my output is entirely different:

test $ npm run compile

> [email protected] compile /Users/beckwith/Code/test
> tsc -p .

node_modules/@google-cloud/precise-date/build/src/index.d.ts:87:39 - error TS2304: Cannot find name 'bigint'.

87     constructor(preciseTime: string | bigint | DateTuple | ProtobufDate);
                                         ~~~~~~

node_modules/@google-cloud/precise-date/build/src/index.d.ts:107:20 - error TS2304: Cannot find name 'bigint'.

107     getFullTime(): bigint;
                       ~~~~~~

node_modules/@google-cloud/precise-date/build/src/index.d.ts:196:41 - error TS2304: Cannot find name 'bigint'.

196     setFullTime(time: string | number | bigint): string;
                                            ~~~~~~

node_modules/@google-cloud/precise-date/build/src/index.d.ts:302:37 - error TS2304: Cannot find name 'bigint'.

302     static parseFull(time: string | bigint | DateTuple | ProtobufDate): string;
                                        ~~~~~~

node_modules/@google-cloud/precise-date/build/src/index.d.ts:323:40 - error TS2304: Cannot find name 'bigint'.

323     static fullUTC(...args: number[]): bigint;

@JustinBeckwith JustinBeckwith self-assigned this Mar 12, 2019
@JustinBeckwith
Copy link
Contributor

ok figured it out! It's the version of TypeScript that's probably creating the problem. When I hard code to 3.0.0, this problem appears. If I upgrade to 3.3.3333 (no, that is not a typo...) it all works great.

@marcusnielsen what version of TypeScript are you using?

@callmehiphop
Copy link
Contributor

@JustinBeckwith I'm able to reproduce this consistently with a local install of 3.3.3333.

@JustinBeckwith
Copy link
Contributor

waaaaaaaaat

@callmehiphop
Copy link
Contributor

I think it might be the result of using some of the defaults that tsc --init provides. Here's my tsconfig.json

{
  "compilerOptions": {
    "target": "es5",
    "module": "commonjs",
    "outDir": "./build",
    "rootDir": "./",
    "strict": true,
    "types": ["node"],
    "esModuleInterop": true
  }
}

@marcusradell
Copy link
Author

My report applies to typescript version 3.3.3333.

@cdock1029
Copy link

cdock1029 commented Mar 12, 2019

typescript 3.3.3333

Same error as @marcusnielsen, in gax-nodejs/src/streaming.ts
they are doing:

import * as Duplexify from 'duplexify';

export class StreamProxy extends Duplexify {...}

Edit: removing this is not ideal, causes errors importing other modules.
Making this change in my tsconfig.json eliminated the build error:

{
- "esModuleInterop": true,
+ "esModuleInterop": false,
}

esModuleInterop:

Emit '__importStar' and '__importDefault' helpers for runtime babel ecosystem
compatibility and enable '--allowSyntheticDefaultImports' for typesystem compatibility.
Requires TypeScript version 2.7 or later.

Seems import * as is discouraged:

microsoft/TypeScript#19675

Edit

Try this instead of removing esModuleInterop to isolate fix only to this library:

const PubSub = require('@google-cloud/pubsub').PubSub

@marcusradell
Copy link
Author

I can confirm that it builds with const { PubSub } = require("@google-cloud/pubsub");.

I haven't run any code yet to try it out, but hopefully this unblocks us from using the latest version.

Well done with the speedy work here! 🥇

From my point of view, you can close the issue unless you want to keep it open to solve the root cause.

@callmehiphop callmehiphop added priority: p2 Moderately-important priority. Fix may not be included in next release. and removed priority: p1 Important issue which blocks shipping the next release. Will be fixed prior to next release. labels Mar 13, 2019
@MattGson
Copy link

MattGson commented Apr 4, 2019

Looks like GAX had the same issue but has since solved it by removing the dependency on the duplexify types.
Could the same solution be applied here?

I have a combined JS/TS project and rely on esModuleInterop for a number of other dependencies. Unfortunately using CommonJS imports doesn't solve the problem for me (I was already using it for pub sub).

@marcusradell
Copy link
Author

marcusradell commented Apr 5, 2019

@MattGson do you have the same issue with typescript v3.4.1? We were able to solve it by upgrading typescript. You might want to delete your package-lock.json and rm -rf node_modules just to be safe if you try.

After doing so we have managed to use the normal import syntax in all of our projects without any issues.

EDIT: I just realized that it might not have been related to typescript v3.4.1, but rather the wiping of the lock file.

@MattGson
Copy link

MattGson commented Apr 6, 2019

Hey, that seems to have done the trick. Thanks very much.

I tried deleting package-lock.json and reinstalling with no luck. Then tried updating TS, still no luck. Then I deleted node modules and package-lock.json and reinstalled which seems to have fixed it.

@bcoe
Copy link
Contributor

bcoe commented Apr 8, 2019

@MattGson @marcusnielsen I'm glad you've found a solution to this issue 🎉

@marcusnielsen please feel free to reopen if you continue to have any issues 👍, closing for now.

@bcoe bcoe closed this as completed Apr 8, 2019
@google-cloud-label-sync google-cloud-label-sync bot added the api: pubsub Issues related to the googleapis/nodejs-pubsub API. label Jan 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: pubsub Issues related to the googleapis/nodejs-pubsub API. priority: p2 Moderately-important priority. Fix may not be included in next release. type: bug Error or flaw in code with unintended results or allowing sub-optimal usage patterns.
Projects
None yet
Development

No branches or pull requests

6 participants