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

Add loader for .graphql files #3909

Merged
merged 17 commits into from
Feb 3, 2018
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions packages/react-scripts/config/jest/graphqlTransform.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
// @remove-on-eject-begin
/**
* Copyright (c) 2018-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.
*/
// @remove-on-eject-end
'use strict';

Copy link
Contributor

Choose a reason for hiding this comment

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

Please add Copyright (c) 2016 Remind to the license header of this file, per
https://unpkg.com/[email protected]/LICENSE

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does 39d39bc look correct? Not 100% sure how derivative works should be marked.

const graphqlTransform = require('jest-transform-graphql');

module.exports = graphqlTransform;
5 changes: 5 additions & 0 deletions packages/react-scripts/config/webpack.config.dev.js
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,11 @@ module.exports = {
},
],
},
// The GraphQL loader preprocesses GraphQL queries in .graphql and .gql files.
{
test: /\.(graphql|gql)$/,
Copy link
Contributor

Choose a reason for hiding this comment

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

I prefer us to be opinionated here and pick .graphql as the extension. It's not obvious what .gql is, it has to be repeated everywhere, syntax highlighters have to support both, etc.

Copy link
Contributor Author

@petetnt petetnt Feb 3, 2018

Choose a reason for hiding this comment

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

Not sure if I agree there as both extensions have been adopted by the userland at large from my observations. For example the Apollo references both even while usually using .graphql and VSCode syntax highlighter uses gql in its examples.

I do agree that .graphql is a better extension though.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I see from graphql/graphql-spec#203 that .graphql is used by FB internally so I guess being opinionated is alright there. It will be trivial to add the other extension if needed later

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed in d239f56

loader: 'graphql-tag/loader',
},
// "file" loader makes sure those assets get served by WebpackDevServer.
// When you `import` an asset, you get its (virtual) filename.
// In production, they would get copied to the `build` folder.
Expand Down
5 changes: 5 additions & 0 deletions packages/react-scripts/config/webpack.config.prod.js
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,11 @@ module.exports = {
),
// Note: this won't work without `new ExtractTextPlugin()` in `plugins`.
},
// The GraphQL loader preprocesses GraphQL queries in .graphql and .gql files.
{
test: /\.(graphql|gql)$/,
loader: 'graphql-tag/loader',
},
// "file" loader makes sure assets end up in the `build` folder.
// When you `import` an asset, you get its filename.
// This loader doesn't use a "test" so it will catch all modules
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,21 @@ describe('Integration', () => {
);
});

it('graphql files inclusion', async () => {
const doc = await initDOM('graphql-inclusion');
const children = doc.getElementById('graphql-inclusion').children;

// .graphql
expect(children[0].textContent.replace(/\s/g, '')).to.equal(
'{"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"query","variableDefinitions":[],"directives":[],"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"test"},"arguments":[{"kind":"Argument","name":{"kind":"Name","value":"test"},"value":{"kind":"StringValue","value":"test","block":false}}],"directives":[],"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"test"},"arguments":[],"directives":[]}]}}]}}],"loc":{"start":0,"end":40,"source":{"body":"{\\ntest(test:\\"test\\"){\\ntest\\n}\\n}\\n","name":"GraphQLrequest","locationOffset":{"line":1,"column":1}}}}'
);

// .gql
Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to go

expect(children[1].textContent.replace(/\s/g, '')).to.equal(
'{"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"query","variableDefinitions":[],"directives":[],"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"test"},"arguments":[{"kind":"Argument","name":{"kind":"Name","value":"test"},"value":{"kind":"StringValue","value":"test","block":false}}],"directives":[],"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","name":{"kind":"Name","value":"test"},"arguments":[],"directives":[]}]}}]}}],"loc":{"start":0,"end":40,"source":{"body":"{\\ntest(test:\\"test\\"){\\ntest\\n}\\n}\\n","name":"GraphQLrequest","locationOffset":{"line":1,"column":1}}}}'
);
});

it('image inclusion', async () => {
const doc = await initDOM('image-inclusion');

Expand Down
5 changes: 5 additions & 0 deletions packages/react-scripts/fixtures/kitchensink/src/App.js
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,11 @@ class App extends Component {
this.setFeature(f.default)
);
break;
case 'graphql-inclusion':
import('./features/webpack/GraphQLInclusion').then(f =>
this.setFeature(f.default)
);
break;
case 'image-inclusion':
import('./features/webpack/ImageInclusion').then(f =>
this.setFeature(f.default)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* Copyright (c) 2018-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.
*/

import React from 'react';
import A from './assets/graphql.graphql';
import B from './assets/graphql.gql';

export default () => (
<p id="graphql-inclusion">
<span>{JSON.stringify(A)}</span>
<span>{JSON.stringify(B)}</span>
</p>
);
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* Copyright (c) 2018-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.
*/

import React from 'react';
import ReactDOM from 'react-dom';
import GraphQLInclusion from './GraphQLInclusion';

describe('graphql files inclusion', () => {
it('renders without crashing', () => {
const div = document.createElement('div');
ReactDOM.render(<GraphQLInclusion />, div);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
Copy link
Contributor

Choose a reason for hiding this comment

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

This too

test(test: "test") {
test
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
{
test(test: "test") {
test
}
}
3 changes: 3 additions & 0 deletions packages/react-scripts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,12 @@
"find-pkg": "1.0.0",
"fs-extra": "5.0.0",
"globby": "7.1.1",
"graphql": "0.12.3",
"graphql-tag": "2.6.1",
"html-webpack-plugin": "2.30.1",
"identity-obj-proxy": "3.0.0",
"jest": "22.1.2",
"jest-transform-graphql": "2.1.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

It's kind of frustrating that everybody will "inherit" whole three GQL dependencies after ejecting even if they don't use graphql.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do agree, but not sure what the other option would be.

Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove this dependency. All it does is literally one-line call to webpack loader, and we could do the same in our Jest transform file.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done ✅

"object-assign": "4.1.1",
"postcss-flexbugs-fixes": "3.2.0",
"postcss-loader": "2.0.10",
Expand Down
3 changes: 2 additions & 1 deletion packages/react-scripts/scripts/utils/createJestConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@ module.exports = (resolve, rootDir, srcRoots) => {
transform: {
'^.+\\.(js|jsx|mjs)$': resolve('config/jest/babelTransform.js'),
'^.+\\.css$': resolve('config/jest/cssTransform.js'),
'^(?!.*\\.(js|jsx|mjs|css|json)$)': resolve(
'^.+\\.(gql|graphql)$': resolve('config/jest/graphqlTransform.js'),
'^(?!.*\\.(js|jsx|mjs|css|json|graphql|gql)$)': resolve(
'config/jest/fileTransform.js'
),
},
Expand Down
27 changes: 27 additions & 0 deletions packages/react-scripts/template/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ You can find the most recent version of this guide [here](https://github.com/fac
- [Post-Processing CSS](#post-processing-css)
- [Adding a CSS Preprocessor (Sass, Less etc.)](#adding-a-css-preprocessor-sass-less-etc)
- [Adding Images, Fonts, and Files](#adding-images-fonts-and-files)
- [Adding GraphQL files](#adding-graphql-files)
- [Using the `public` Folder](#using-the-public-folder)
- [Changing the HTML](#changing-the-html)
- [Adding Assets Outside of the Module System](#adding-assets-outside-of-the-module-system)
Expand Down Expand Up @@ -729,6 +730,32 @@ Please be advised that this is also a custom feature of Webpack.
**It is not required for React** but many people enjoy it (and React Native uses a similar mechanism for images).<br>
An alternative way of handling static assets is described in the next section.

## Adding GraphQL files

Copy link
Contributor

Choose a reason for hiding this comment

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

This needs to have a note about it being supported only in 2.0.0+, like other similar features.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done ✅

If you are using GraphQL, you can **`import` GraphQL files in a JavaScript module**.

By importing GraphQL queries instead of using a [template tag](https://github.com/apollographql/graphql-tag), they are preprocessed at build time. This eliminates the need to process them on the client at run time. It also allows you to separate your GraphQL queries from your code. You can put a GraphQL query in a file with a `.graphql` or `.gql` extension.

Here is an example:

```js
// query.graphql
{
githubStats(repository: "facebook/react") {
stars
}
}

// foo.js

import query from './query.graphql';

console.log(query);
// {
// "kind": "Document",
// ...
```

## Using the `public` Folder

>Note: this feature is available with `[email protected]` and higher.
Expand Down