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

[bug] does not work with @angular/angular-cli@6.0.0 #1344

Closed
amitport opened this issue May 10, 2018 · 13 comments
Closed

[bug] does not work with @angular/angular-cli@6.0.0 #1344

amitport opened this issue May 10, 2018 · 13 comments

Comments

@amitport
Copy link

The core issue is the same as angular/angular-cli#10744
I'm not sure the fix should be only in @angular-devkit- to me it seems reasonable to expect that graphql lib provide a .js main file.

There is an additional complication. After editing @angular-devkit trying to fix .mjs issue according to #1272 (comment), I'm getting the following error:

instanceOf.mjs:3 Uncaught ReferenceError: process is not defined
    at Object../node_modules/graphql/jsutils/instanceOf.mjs (instanceOf.mjs:3)
    at __webpack_require__ (bootstrap:81)
    at Object../node_modules/graphql/type/definition.mjs (vendor.js:151316)
    at __webpack_require__ (bootstrap:81)
    at Object../node_modules/graphql/type/validate.mjs (vendor.js:153362)
    at __webpack_require__ (bootstrap:81)
    at Object../node_modules/graphql/graphql.mjs (vendor.js:146722)
    at __webpack_require__ (bootstrap:81)
    at Object../node_modules/graphql/index.mjs (vendor.js:146842)
    at __webpack_require__ (bootstrap:81)
@Ben305
Copy link

Ben305 commented May 15, 2018

This is a blocking issue for us. Is there any ETA for a fix?

@IvanGoncharov
Copy link
Member

IvanGoncharov commented May 15, 2018

@Ben305 It's issue with angular-cli that is already fixed see here:
angular/angular-cli#10744 (comment)

@amitport Since this issue already resolved by the change in angular-cli so can I close it?

@amitport
Copy link
Author

amitport commented May 16, 2018

@IvanGoncharov can we wait a couple of days until it will be included in some release? make sure everything is working OK together.

EDIT: I'm still getting the instanceOf.mjs:3 Uncaught ReferenceError: process is not defined part. This lib still doesn't work with Angular@6.

EDIT 2: this is an intentionally breaking change in Angular-CLI
angular/angular-cli#9827 (comment)
"Browser code should not rely on things that are not available in browser environments."

(also related: angular/angular-cli#10681 (comment))

@IvanGoncharov
Copy link
Member

can we wait a couple of days until it will be included in some release?

@amitport Sure 👍

@IvanGoncharov
Copy link
Member

I'm still getting the instanceOf.mjs:3 Uncaught ReferenceError: process is not defined part. This lib still doesn't work with Angular@6.

@amitport It looks like this was fixed in 60c03ab but not released yet. Can you please run your test on master, you can use built version from here:
https://github.com/graphql/graphql-js#want-to-ride-the-bleeding-edge

@amitport
Copy link
Author

amitport commented May 17, 2018

Did work, though I got to say IMO it's missing the point made in angular/angular-cli#9827 (comment).

A library should not rely on a specific build tool, and in this case it relies on webpack to replace the process.env.NODE_ENV === 'production' block. In this specific case, I would remove the instanceof wrap entirely for the same reason Angular removed process wrap. Shouldn't hide the real problem which is with npm/yarn (a usability problem, not a bug). Plenty of other libs have the same concern.

This is not relevant to this issue though. Closing, thanks!

@Ben305
Copy link

Ben305 commented May 18, 2018

@amitport Can confirm this works with @angular/cli 6.0.3 and latest master branch. Can we get a new release on NPM please?

@amitport
Copy link
Author

@Ben305 worked for me with @angular-devkit/build-angular@0.6.3 (Angular 6+ version is irrelevant) and graphql@master. 👍 for new graphql release (@leebyron ? not sure who should be tagged here :/ )

@Ben305
Copy link

Ben305 commented May 19, 2018

Maybe @mjmahone could also publish a new release with this fix

@IvanGoncharov
Copy link
Member

@Ben305 @amitport Update: @mjmahone working on 0.14.0 release, see here: #1346 (comment)

@DmitryEfimenko
Copy link

when is this expected to be released?

@danutzcodrescu
Copy link

I have update to graphql 14 and it solved the issue.

@Cybernetixs
Copy link

Yes, its a graphql issue, had the same problem with React@16, had to upgrade to graphql 14

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

No branches or pull requests

6 participants