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

Build .mjs files (ES modules) alongside CommonJS #209

Closed
wants to merge 11 commits into from
2 changes: 1 addition & 1 deletion .babelrc
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@
"transform-es2015-parameters",
["transform-es2015-destructuring", {loose: true}],
"transform-es2015-block-scoping",
"transform-es2015-modules-commonjs",
"./resources/common-js-modules",
"transform-regenerator",
]
}
2 changes: 1 addition & 1 deletion .flowconfig
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@
[options]

[version]
^0.69.0
^0.73.0
11 changes: 8 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,9 @@
"type": "git",
"url": "http://github.com/graphql/graphql-relay-js.git"
},
"main": "lib/index.js",
"main": "lib/index",
"module": "lib/index.mjs",
"sideEffects": false,
"directories": {
"lib": "./lib"
},
Expand All @@ -32,7 +34,10 @@
"testonly": "babel-node ./node_modules/.bin/_mocha $npm_package_options_mocha",
"lint": "eslint src",
"check": "flow check",
"build": "rm -rf lib/* && babel src --ignore __tests__ --out-dir lib && npm run build:flow",
"build": "npm run build:clean && npm run build:cjs && npm run build:mjs && npm run build:flow",
"build:clean": "rm -rf ./lib && mkdir ./lib",
"build:cjs": "babel src --ignore __tests__ --out-dir lib",
"build:mjs": "BABEL_MODULES=1 babel src --optional runtime --ignore __tests__ --out-dir lib/module/ && for file in $(find lib/module -name '*.js'); do mv \"$file\" `echo \"$file\" | sed 's/lib\\/module/lib/g; s/.js$/.mjs/g'`; done && rm -rf lib/module",
"build:flow": "find ./src -name '*.js' -not -path '*/__tests__*' | while read filepath; do cp $filepath `echo $filepath | sed 's/\\/src\\//\\/lib\\//g'`.flow; done",
"watch": "babel-node scripts/watch.js",
"cover": "babel-node node_modules/.bin/isparta cover --root src --report html node_modules/.bin/_mocha -- $npm_package_options_mocha",
Expand Down Expand Up @@ -71,7 +76,7 @@
"eslint": "^4.17.0",
"eslint-plugin-babel": "^4.1.2",
"eslint-plugin-flowtype": "^2.44.0",
"flow-bin": "^0.69.0",
"flow-bin": "^0.73.0",
"graphql": "^0.13.1",
"isparta": "4.0.0",
"mocha": "^5.0.1",
Expand Down
17 changes: 17 additions & 0 deletions resources/common-js-modules.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/

'use strict';

/**
* Determines whether to transform modules to commonjs based on an
* environment variable. Module import/export statements are not transformed
* if the `BABEL_MODULES` env variable is set.
*/
module.exports = process.env.BABEL_MODULES ?
() => ({}) :
require('babel-plugin-transform-es2015-modules-commonjs');
2 changes: 1 addition & 1 deletion src/__tests__/starWarsConnectionTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import { expect } from 'chai';
import { describe, it } from 'mocha';
import { StarWarsSchema } from './starWarsSchema.js';
import { StarWarsSchema } from './starWarsSchema';
Copy link
Member

@IvanGoncharov IvanGoncharov May 26, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@motiz88 If you have time maybe it worth to split such changes into separate PR.
I think it would be easy to find someone from Facebook willing to merge such simple change
+ it will make the main PR smaller

import { graphql } from 'graphql';

// 80+ char lines are useful in describe/it, so ignore in this file.
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/starWarsMutationTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import { expect } from 'chai';
import { describe, it } from 'mocha';
import { StarWarsSchema } from './starWarsSchema.js';
import { StarWarsSchema } from './starWarsSchema';
import { graphql } from 'graphql';

// 80+ char lines are useful in describe/it, so ignore in this file.
Expand Down
2 changes: 1 addition & 1 deletion src/__tests__/starWarsObjectIdentificationTests.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import { expect } from 'chai';
import { describe, it } from 'mocha';
import { StarWarsSchema } from './starWarsSchema.js';
import { StarWarsSchema } from './starWarsSchema';
import { graphql } from 'graphql';

// 80+ char lines are useful in describe/it, so ignore in this file.
Expand Down
10 changes: 5 additions & 5 deletions src/__tests__/starWarsSchema.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,28 +19,28 @@ import {
nodeDefinitions,
globalIdField,
fromGlobalId
} from '../node/node.js';
} from '../node/node';

import {
connectionFromArray
} from '../connection/arrayconnection.js';
} from '../connection/arrayconnection';

import {
connectionArgs,
connectionDefinitions
} from '../connection/connection.js';
} from '../connection/connection';

import {
mutationWithClientMutationId
} from '../mutation/mutation.js';
} from '../mutation/mutation';

import {
getFaction,
getShip,
getRebels,
getEmpire,
createShip,
} from './starWarsData.js';
} from './starWarsData';

/**
* This is a basic end-to-end test, designed to demonstrate the various
Expand Down
4 changes: 2 additions & 2 deletions src/connection/__tests__/connection.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@ import {

import {
connectionFromArray,
} from '../arrayconnection.js';
} from '../arrayconnection';

import {
backwardConnectionArgs,
connectionArgs,
connectionDefinitions,
forwardConnectionArgs,
} from '../connection.js';
} from '../connection';

import { expect } from 'chai';
import { describe, it } from 'mocha';
Expand Down
2 changes: 1 addition & 1 deletion src/connection/arrayconnection.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import type {
import {
base64,
unbase64,
} from '../utils/base64.js';
} from '../utils/base64';

type ArraySliceMetaInfo = {
sliceStart: number;
Expand Down
14 changes: 7 additions & 7 deletions src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,15 @@ export type {
ConnectionCursor,
Edge,
PageInfo,
} from './connection/connectiontypes.js';
} from './connection/connectiontypes';

// Helpers for creating connection types in the schema
export {
backwardConnectionArgs,
connectionArgs,
connectionDefinitions,
forwardConnectionArgs,
} from './connection/connection.js';
} from './connection/connection';

// Helpers for creating connections from arrays
export {
Expand All @@ -34,26 +34,26 @@ export {
cursorToOffset,
getOffsetWithDefault,
offsetToCursor,
} from './connection/arrayconnection.js';
} from './connection/arrayconnection';

// Helper for creating mutations with client mutation IDs
export {
mutationWithClientMutationId,
} from './mutation/mutation.js';
} from './mutation/mutation';

// Helper for creating node definitions
export {
nodeDefinitions,
} from './node/node.js';
} from './node/node';

// Helper for creating plural identifying root fields
export {
pluralIdentifyingRootField,
} from './node/plural.js';
} from './node/plural';

// Utilities for creating global IDs in systems that don't have them.
export {
fromGlobalId,
globalIdField,
toGlobalId,
} from './node/node.js';
} from './node/node';
2 changes: 1 addition & 1 deletion src/node/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ import type {
import {
base64,
unbase64
} from '../utils/base64.js';
} from '../utils/base64';

type GraphQLNodeDefinitions = {
nodeInterface: GraphQLInterfaceType,
Expand Down
2 changes: 2 additions & 0 deletions src/utils/base64.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
* @flow
*/

import { Buffer } from './buffer';

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I understand facebook/flow#3723 was merged and will be available in the next release.
Does that mean you can just change this line to:

import Buffer from 'buffer';

and everything will work?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sadly no. We'd be able to simplify buffer.js#L28-L37 to

export const Buffer = NodeBuffer.Buffer;

(which should now typecheck)

but we'd still have Node's actual implementation to deal with.

Under node --experimental-modules, import { Buffer } from 'buffer' fails because the buffer module doesn't export { Buffer } at all; it only has a default export with Buffer as a property. Hence, the rest of buffer.js is still needed for compatibility with both Node and Webpack, even once the Flow fix lands.

export type Base64String = string;

export function base64(i: string): Base64String {
Expand Down
32 changes: 32 additions & 0 deletions src/utils/buffer.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* Copyright (c) 2015-present, Facebook, Inc.
*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*
* @flow
*/

/**
* This file exists to work around an issue with importing Buffer across
* Webpack and Node.
*
* Webpack (tested with 4.5.0) doesn't currently support use of Node globals,
* such as Buffer, in ES modules.
* See https://github.com/webpack/webpack/issues/7032
*
* Hence we want to import it explicitly e.g.
*
* import { Buffer } from 'buffer';
*
* But Node (tested with version 9.11.1 and --experimental-modules) only has
* Buffer as a property of the default export, not as a separate named export.
*/

import NodeBuffer from 'buffer';

/**
* An alias for Node's Buffer class.
*/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@motiz88 Flow 0.72 released and it includes your fix 🎉 Can you please update this PR?


export const Buffer = NodeBuffer.Buffer;
6 changes: 3 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -1384,9 +1384,9 @@ flat-cache@^1.2.1:
graceful-fs "^4.1.2"
write "^0.2.1"

flow-bin@^0.69.0:
version "0.69.0"
resolved "https://registry.yarnpkg.com/flow-bin/-/flow-bin-0.69.0.tgz#053159a684a6051fcbf0b71a2eb19a9679082da6"
flow-bin@^0.73.0:
version "0.73.0"
resolved "https://registry.yarnpkg.com/flow-bin/-/flow-bin-0.73.0.tgz#da1b90a02b0ef9c439f068c2fc14968db83be425"

for-in@^1.0.1, for-in@^1.0.2:
version "1.0.2"
Expand Down