Shouldn't graphiql (in browser ide for exploring graphql) and sqlite3 be under devDependencies instead of dependencies in package.json?
@buildbreakdo do you think you will never need to use GraphiQL on the production server? Do you think GraphQL/SQL/Auth should be removed from the RSK and moved into a separate project (though, this may complicate deployment)?
Currently sqlite3 package must go to regular dependencies because it's supposed to be used at runtime on the server. When you deploy your app, most likely you will run npm install --production to make sure that run-time dependencies are ready to be used by the Node.js app and don't spend server resources installing dev dependencies.
I have already separated API (GraphQL/SQL/Auth) from my clone of the starter kit. Not because I prefer it this way, but because it's a necessary for my use case.
My API needs to serve more than 1 website built using this starter kit and obviously it's not a good idea for each of them to have a separate API. As an added bonus my transpile times are much faster now since I'm working either on the API or the starter kit at any given time.
Deployment will be more difficult yes, I need to think how to handle the intl messages for each site, I'll probably put them into API's database.
@koistya GraphiQL on prod -- what are the production use cases? Dev ones are clear:
Only one that seems to possibly fit production is the last one? Maybe if a developer wanted to inspect data in a production env? Exceptional case? Was just thinking do users/customers really need access to an in browser ide for exploring graphql? Could be interesting from an admin portal perspective but for the general scenario, including the module in prod seems like mixing tools used to develop into an environment (production) that is all about consumption.
Would keep GraphQL/SQL/Auth as a part of the RSK. In my experience, majority of developers are creating CRUD apps, believe RSK demonstrating this functionality out of the box lets the decision maker know that this kit matches (or at least considers) CRUD needs. Think this is why we see a lot of To Do list examples (persistence/CRUD example). Think the RSK needs something like this too.
@buildbreakdo that makes sense, let's remove GraphiQL from the production build.
@koistya Now this is interesting. Started to look into this and thought it would be an easy npm uninstall graphiql --save && npm install graphiql --save-dev. However when running npm start -- --release and navigating to /graphql was still seeing GraphiQL IDE.
Turns out this was a double include in a very strange way. GraphiQL is being brought in by express-graphql via a CDN (see here). In node_modules/express-graphql/dist/renderGraphiQL.js you can see it creates the HTML and which includes a CDN script reference to a particular GraphiQL version:
function renderGraphiQL(data) {
var queryString = data.query;
var variablesString = data.variables ? JSON.stringify(data.variables, null, 2) : null;
var resultString = data.result ? JSON.stringify(data.result, null, 2) : null;
var operationName = data.operationName;
/* eslint-disable max-len */
return '<!--\nThe request to this GraphQL server provided the header "Accept: text/html"\nand as a result has been presented GraphiQL - an in-browser IDE for\nexploring GraphQL.\n\nIf you wish to receive JSON, provide the header "Accept: application/json" or\nadd "&raw" to the end of the URL within a browser.\n-->\n<!DOCTYPE html>\n<html>\n<head>\n <meta charset="utf-8" />\n <title>GraphiQL</title>\n <meta name="robots" content="noindex" />\n <style>\n html, body {\n height: 100%;\n margin: 0;\n overflow: hidden;\n width: 100%;\n }\n </style>\n <link href="//cdn.jsdelivr.net/graphiql/' + GRAPHIQL_VERSION + '/graphiql.css" rel="stylesheet" />\n <script src="//cdn.jsdelivr.net/fetch/0.9.0/fetch.min.js"></script>\n <script src="//cdn.jsdelivr.net/react/15.0.0/react.min.js"></script>\n <script src="//cdn.jsdelivr.net/react/15.0.0/react-dom.min.js"></script>\n <script src="//cdn.jsdelivr.net/graphiql/' + GRAPHIQL_VERSION + '/graphiql.min.js"></script>\n</head>\n<body>\n <script>\n // Collect the URL parameters\n var parameters = {};\n window.location.search.substr(1).split(\'&\').forEach(function (entry) {\n var eq = entry.indexOf(\'=\');\n if (eq >= 0) {\n parameters[decodeURIComponent(entry.slice(0, eq))] =\n decodeURIComponent(entry.slice(eq + 1));\n }\n });\n\n // Produce a Location query string from a parameter object.\n function locationQuery(params) {\n return \'?\' + Object.keys(params).map(function (key) {\n return encodeURIComponent(key) + \'=\' +\n encodeURIComponent(params[key]);\n }).join(\'&\');\n }\n\n // Derive a fetch URL from the current URL, sans the GraphQL parameters.\n var graphqlParamNames = {\n query: true,\n variables: true,\n operationName: true\n };\n\n var otherParams = {};\n for (var k in parameters) {\n if (parameters.hasOwnProperty(k) && graphqlParamNames[k] !== true) {\n otherParams[k] = parameters[k];\n }\n }\n var fetchURL = locationQuery(otherParams);\n\n // Defines a GraphQL fetcher using the fetch API.\n function graphQLFetcher(graphQLParams) {\n return fetch(fetchURL, {\n method: \'post\',\n headers: {\n \'Accept\': \'application/json\',\n \'Content-Type\': \'application/json\'\n },\n body: JSON.stringify(graphQLParams),\n credentials: \'include\',\n }).then(function (response) {\n return response.text();\n }).then(function (responseBody) {\n try {\n return JSON.parse(responseBody);\n } catch (error) {\n return responseBody;\n }\n });\n }\n\n // When the query and variables string is edited, update the URL bar so\n // that it can be easily shared.\n function onEditQuery(newQuery) {\n parameters.query = newQuery;\n updateURL();\n }\n\n function onEditVariables(newVariables) {\n parameters.variables = newVariables;\n updateURL();\n }\n\n function onEditOperationName(newOperationName) {\n parameters.operationName = newOperationName;\n updateURL();\n }\n\n function updateURL() {\n history.replaceState(null, null, locationQuery(parameters));\n }\n\n // Render <GraphiQL /> into the body.\n ReactDOM.render(\n React.createElement(GraphiQL, {\n fetcher: graphQLFetcher,\n onEditQuery: onEditQuery,\n onEditVariables: onEditVariables,\n onEditOperationName: onEditOperationName,\n query: ' + safeSerialize(queryString) + ',\n response: ' + safeSerialize(resultString) + ',\n variables: ' + safeSerialize(variablesString) + ',\n operationName: ' + safeSerialize(operationName) + ',\n }),\n document.body\n );\n </script>\n</body>\n</html>';
}
Clever way to conditionally include a module! (although it does require an internet connection to use the GraphiQL IDE, which I don't like at all). Effectively acts as an if statement around an import.
Looks like there isn't a way to configure express-graphql to use an npm module instead of their CDN, unless I am missing something. Do believe we should be able to disconnect from the internet and still have a full development environment (including the GraphiQL IDE) -- looks like that's not currently possible with the express-graphql.
Confirmed also that express-graphql won't automatically pick up GraphiQL if npm install graphiql --save-dev is run, so basically, there is no reason to include GraphiQL as separate module in dev or prod.
So for now, we remove the GraphiQL IDE module altogether because it is just dead weight (not being used at all) and add a production conditional for GraphiQL in server.js.
app.use('/graphql', expressGraphQL(req => ({
schema,
graphiql: process.env.NODE_ENV !== 'production',
rootValue: { request: req },
pretty: process.env.NODE_ENV !== 'production',
})));
Will open a separate issue to discuss having a full development offline with the RSK (frequently work on planes, buses, and trains so sensitive to this issue).