Add undefined export definitions to make rollup happy

[gaberudy]

To fix #1293

[facebook-github-bot]

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed.

If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks!

[facebook-github-bot]

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

[acoreyj]

Can confirm this fixes my issue with using graphql and rollup (as part of Stenciljs project)

[gaberudy]

@acoreyj I'm curious if you also had this issue (which I can't tell whether its a problem with graphql, rollup or the rollup-commonjs-plugin):

rollup/rollup#1859

[acoreyj]

@gaberudy

Hmm no I haven't had that issue in my project

[leebyron]

The npm branch is automatically maintained as the compiled output of the source master branch. Please make changes against the master branch.

[gaberudy]

@leebyron that may be the case, but the transpiling is incorrect in this case.

For some reason, the validation sub-module gets correct codegen (that's what I used as a template to fix the other sub-modules).

My only guess is that it pulls in a default import, which may anchor the transpiler to enumerate in that scope all the export variables instead of doing just the Object.defineProperty on them.

I suspect that:

graphql-js/src/validation/index.js

Line 13 in 920fb87

import ValidationContext from './ValidationContext';

Maybe be responsible for having the correct transpiled code. Not sure if there is a way to fake this in the other sub-modules..

[leebyron]

I'm not sure I understand why the compiled output is incorrect - do you have a test case that fails when loading it in? In the node repl I can require the module and submodules and get access to all exported values.

I believe Babel adds additional code to the validation submodule due to require statements being interleaved in the result. The additional code just initializes the export as undefined before the call to Object.defineProperty in case any interleaved require statement happens to cause a circular require, then the property will be defined on the exported object. The submodules that don't have this pattern don't have that additional code since the property cannot be accessed before Object.defineProperty. If I recall, Babel v7 changed that behavior slightly - so we may expect the compiled output to change when adopting Babel v7, however the runtime behavior should be identical.

[gaberudy]

Yea, I see it's a what you are saying ,and man, your knowledge of Babel internal transpiling behavior is impressive.

Anyway, this shouldn't be accepted, and hopefully the rollup upstream maintainers see the shiny new way of the mjs side-by-side modules definitions and then dealing with the transpiled files becomes mute.