feat(codegen): disallow src in fileName #87

d4rekanguok · 2020-04-15T09:44:19Z

Edit: Removed useModule, this PR now only add new improvements for the existing fileName option.

Update:

fileName

Outputing into src directory is now disallowed.

i.e

// gatsby-config.js

module.exports = {
  plugins: [{
    resolve: 'gatsby-plugin-graphql-codegen',
    options: { fileName: 'src/types.ts' }
  }]
}

// throw error

Dev stuff:

gatsby-plugin-graphql-codegen now use webpack to bundle into a single gatsby-node.js.

d4rekanguok · 2020-04-15T09:45:40Z

Todos:

make it work with additionalSchema
gatsby-starter-ts is broken for whatever reasons, gatsby doesn't recognize src/pages :(

d4rekanguok · 2020-04-15T09:46:55Z

@ricokahler let me know what you think about these new options!

d4rekanguok · 2020-04-16T15:13:35Z

Need to reconsider this in light of @cometkim comment in #87

It looks like yarn 2 can understand good ol' node_modules in loose mode, but I don't wanna risk the typescript slow update. Perhaps we could use just set a path alias in tsconfig.json?

d4rekanguok · 2020-05-07T05:38:49Z

Updated to remove the useModule stuff. Will close #85 when merged

@rico please take a look!

ricokahler

Nice improvement! I think this will make the DX of the lib a lot better.

I think i caught few smaller things and I'm not sure I understand the benefit of bundling via webpack. Let me know what you think!

ricokahler · 2020-05-09T05:42:00Z

packages/gatsby-starter-blog-ts/src/pages/index.tsx

@@ -6,7 +6,7 @@ import Layout from '../components/layout'
 import SEO from '../components/seo'
 import { rhythm } from '../utils/typography'

-import { BlogIndexQuery } from '../../gen/graphql-types'
+import { BlogIndexQuery } from 'gatsby-ts'


is this still valid? I didn't see anything that would enable importing like this.

shoot, thanks for catching this 🤦‍♀️

ricokahler · 2020-05-09T05:42:19Z

packages/gatsby-starter-blog-ts/src/pages/index.tsx

+    !data ||
+    !data.site ||
+    !data.site.siteMetadata ||
+    !data.site.siteMetadata.title ||
+    !data.allMarkdownRemark
+  ) {
+    throw new Error('no data')
+  }
+  const siteTitle = data.site.siteMetadata.title
+  const posts = data.allMarkdownRemark.edges


old code but optional chaining would be nice here.

ricokahler · 2020-05-09T05:42:36Z

packages/gatsby-starter-ts/src/components/seo.tsx

@@ -9,7 +9,7 @@ import * as React from 'react'
 import Helmet from 'react-helmet'
 import { useStaticQuery, graphql } from 'gatsby'
 import ensureKeys from '../helpers/ensure-keys'
-import { SeoQuery } from '../../graphql-types'
+import { SeoQuery } from 'gatsby-ts'


same comment

ricokahler · 2020-05-09T05:42:43Z

packages/gatsby-starter-ts/src/components/layout.tsx

@@ -8,7 +8,7 @@
 import * as React from 'react'
 import { useStaticQuery, graphql } from 'gatsby'
 import ensureKeys from '../helpers/ensure-keys'
-import { SiteTitleQuery } from '../../graphql-types'
+import { SiteTitleQuery } from 'gatsby-ts'


same comment

ricokahler · 2020-05-09T05:42:49Z

packages/gatsby-starter-ts/src/components/image.tsx

@@ -2,7 +2,7 @@ import * as React from 'react'
 import { useStaticQuery, graphql } from 'gatsby'
 import Img from 'gatsby-image'
 import ensureKeys from '../helpers/ensure-keys'
-import { ImageQuery } from '../../graphql-types'
+import { ImageQuery } from 'gatsby-ts'


same comment

ricokahler · 2020-05-09T05:56:47Z

packages/gatsby-plugin-graphql-codegen/package.json

-    "typescript": "^3.7.5"
+    "ts-loader": "^7.0.0",
+    "typescript": "^3.7.5",
+    "webpack": "^4.42.1",


Curious: why did you decide to bundle this package webpack? Since these packages are meant for node, I don't see much value in bundling. And if you do decide to bundle, any reason why webpack over rollup?

gatsby-node has to be placed in the root, otherwise Gatsby can't find it. Because of this, all files inside src is compiled to the root dir. I didn't want to ship unnecessary files, so I've been manually add/remove files to .npmignore. This is quite cumbersome as the amount of files increase.

Rollup was no.1 choice, but for whatever reasons I couldn't make it work with various typescript plugins. I had little time, so I decided to go with webpack, which worked first try.

Now come to think about it, I've realized I don't really need to bundle, I can just npmignore the src dir 🤦‍♀️ Or just ship the whole thing.

ricokahler · 2020-05-09T06:05:11Z

packages/gatsby-plugin-graphql-codegen/webpack.config.js

+  },
+  externals: [
+    {
+      'fs-extra': 'commonjs2 fs-extra',


What does putting commonjs before the package here do? I've never seen this before.

It's additional context so webpack understand that the module (fs-extra) is a commonjs module, docs here. It's only useful for bundling node stuff. Without commonjs2, webpack would look for fs-extra in global space

ricokahler · 2020-05-09T06:06:26Z

packages/gatsby-plugin-graphql-codegen/src/utils/is-in-src.ts

+type IsInSrc = (srcDir: string, fileDir: string) => boolean
+export const isInSrc: IsInSrc = (srcDir, fileDir) => {
+  const relativePath = path.relative(srcDir, fileDir)
+  return !relativePath.startsWith('..')


ah this is smart!

ricokahler · 2020-05-09T06:07:24Z

packages/gatsby-plugin-graphql-codegen/.gitignore

+graphql-codegen.config.d.ts
+utils


why is this gitignored?

Now this is why having a second pair of eyes is so valuable, this is probably meant for .npmignore but my 2am brain confused it with .gitignore, which is wrong anyway 🤦‍♀️

ricokahler · 2020-05-09T06:11:46Z

packages/gatsby-plugin-graphql-codegen/src/graphql-codegen.config.test.js

  expect(fs.writeFile).toMatchInlineSnapshot(`
 [MockFunction] {
  "calls": Array [
    Array [
-      "example-directory/example-types.ts",
+      undefined,


i'm very curious on why this changed to undefined. this seems unexpected. can you confirm this is correct?

You're right this isn't right, I'll run the test again

d4rekanguok · 2020-05-11T04:02:09Z

Thanks for reviewing @ricokahler, I shouldn't had tried to do more than one thing in one PR! I'll remove the bundle step & just use a normal .npmignore pattern.

vimtor · 2020-10-03T20:17:40Z

Maybe it is easier to add a way to ignore certain files so avoid an infinite loop?

d4rekanguok added 8 commits April 14, 2020 14:27

check filePath onPreInit

607393e

disallow src

003aaf2

bundle into one file

a8aeeb8

useModule

1cdb2b0

simplify the code + console msg a bit

e981c3d

check if module already exists

7975fb0

update test

ee12d89

remove global constant, add tests

25023bf

d4rekanguok requested a review from ricokahler April 15, 2020 09:44

remove useModule

4462689

d4rekanguok changed the title ~~feat(codegen): generate into node_modules, disallow src~~ feat(codegen): disallow src in fileName May 7, 2020

update test

6af85c1

d4rekanguok added 2 commits May 7, 2020 14:47

undo changes to starter-config

ef9cfb2

invoke codegen onPreInit in gatsby-plugin-ts as well

2bd0c86

ricokahler reviewed May 9, 2020

View reviewed changes

d4rekanguok closed this May 30, 2021

d4rekanguok deleted the feat/module-name branch May 30, 2021 16:01

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(codegen): disallow src in fileName #87

feat(codegen): disallow src in fileName #87

d4rekanguok commented Apr 15, 2020 •

edited

Loading

d4rekanguok commented Apr 15, 2020

d4rekanguok commented Apr 15, 2020

d4rekanguok commented Apr 16, 2020

d4rekanguok commented May 7, 2020 •

edited

Loading

ricokahler left a comment

ricokahler May 9, 2020

d4rekanguok May 11, 2020

ricokahler May 9, 2020

ricokahler May 9, 2020

ricokahler May 9, 2020

ricokahler May 9, 2020

ricokahler May 9, 2020

d4rekanguok May 11, 2020

ricokahler May 9, 2020

d4rekanguok May 11, 2020

ricokahler May 9, 2020

ricokahler May 9, 2020

d4rekanguok May 11, 2020

ricokahler May 9, 2020

d4rekanguok May 11, 2020

d4rekanguok commented May 11, 2020

vimtor commented Oct 3, 2020

		graphql-codegen.config.d.ts
		utils

feat(codegen): disallow src in fileName #87

feat(codegen): disallow src in fileName #87

Conversation

d4rekanguok commented Apr 15, 2020 • edited Loading

Update:

fileName

d4rekanguok commented Apr 15, 2020

d4rekanguok commented Apr 15, 2020

d4rekanguok commented Apr 16, 2020

d4rekanguok commented May 7, 2020 • edited Loading

ricokahler left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

d4rekanguok commented May 11, 2020

vimtor commented Oct 3, 2020

d4rekanguok commented Apr 15, 2020 •

edited

Loading

d4rekanguok commented May 7, 2020 •

edited

Loading