feat(gatsby): Store site metadata #26162

ascorbic · 2020-07-31T10:08:25Z

This creates a metadata file in the config directory, which allows reverse lookup to find the site's path. This is so that Desktop can iterate the directory to discover all sites on the user's drive.

gatsby-cloud · 2020-07-31T10:11:06Z

Gatsby Cloud Build Report

using-styled-components

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 19s

Performance

Lighthouse report

Metric	Score
Performance	💚 100
Accessibility	💚 90
Best Practices	💚 100
SEO	💚 90

🔗 View full report

gatsby-cloud · 2020-07-31T10:11:08Z

Gatsby Cloud Build Report

client-only-paths

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 18s

Performance

Lighthouse report

Metric	Score
Performance	💚 100
Accessibility	🔶 85
Best Practices	💚 100
SEO	🔶 70

🔗 View full report

gatsby-cloud-staging · 2020-07-31T10:11:27Z

Gatsby Cloud Build Report

gatsby-master

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 1m

Performance

Lighthouse report

Metric	Score
Performance	💚 97
Accessibility	🔶 87
Best Practices	💚 93
SEO	🔶 73

🔗 View full report

gatsby-cloud · 2020-07-31T10:12:22Z

Gatsby Cloud Build Report

using-reach-skip-nav

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 21s

Performance

Lighthouse report

Metric	Score
Performance	💚 100
Accessibility	💚 100
Best Practices	💚 100
SEO	🔶 82

🔗 View full report

gatsby-cloud-staging · 2020-07-31T10:29:19Z

Gatsby Cloud Build Report

gatsby

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 22m

Performance

Lighthouse report

Metric	Score
Performance	💚 93
Accessibility	💚 100
Best Practices	💚 100
SEO	🔶 76

🔗 View full report

gatsby-cloud · 2020-07-31T10:35:18Z

Gatsby Cloud Build Report

gatsby

🎉 Your build was successful! See the Deploy preview here.

Build Details

View the build logs here.

🕐 Build time: 19m

mxstbr · 2020-07-31T11:05:16Z

packages/gatsby-core-utils/src/service-lock.ts

-  serviceName: string
-): Promise<string | null> => {
+  serviceName: string,
+  ignoreLockfile: boolean = false


Doesn't look like the new parameter is used anywhere? 🤔

Look at line 81

I meant from the outside 😅 the getService call below doesn't seem to leverage it:

const data = await getService(program.directory, `developproxy`)

Oh, that's for me in Desktop! I want to get the data without obtaining a lock

KyleAMathews · 2020-07-31T18:00:12Z

Can we register the site when it gets created as well as part of gatsby new?

mxstbr · 2020-07-31T18:04:26Z

I reckon we could register in gatsby-cli no matter what command is run, right?

ascorbic · 2020-08-03T09:19:07Z

I've added support for gatsby new. I don't think it makes sense to add it whatever command is run, because quite a lot of commands don't require to be run in the context of a local gatsby site. It might make sense to add it for build.

KyleAMathews · 2020-08-03T15:15:14Z

build makes sense. In my poking around in telemetry, there's definitely people who just clone a site and try building it — e.g. a site a colleague asks them to look at.

ascorbic · 2020-08-03T16:34:18Z

@KyleAMathews @mxstbr OK, it now registers on new, develop and build.

mxstbr

Overall LGTM, one clarification comment inline but that shouldn't be blocking! 👍

mxstbr · 2020-08-04T06:42:30Z

packages/gatsby/src/commands/build.ts

+  await createServiceLock(program.directory, `metadata`, {
+    name: program.sitePackageJson.name,
+    sitePath: program.directory,
+    lastRun: Date.now(),


Is there a reason why this doesn't include pid, but the develop one does? I'm assuming this doesn't store the pid since it's not a long-lived process, but just wanted to clarify!

Yup, exactly. I only include the pid in case I need to kill the process from Desktop.

It doesn't harm us to keep it consistent and always add a pid, right? Do we also want to keep track of which method it is? (build/develop)

Does it make sense to add a typescript type for it?

Moving it to core-utils would solve this as well.

@wardpeet We can't, because the calling process might not always be the actual build process. We use it in Desktop to write the metadata for sites that we have manually added.

packages/gatsby-cli/src/init-starter.ts

sidharthachatterjee

Is there a blanket cleanup necessary for cleaning up these locks if someone sends a SIGINT at a certain time that results in stale lock files lying around?

ascorbic · 2020-08-04T08:05:08Z

@sidharthachatterjee Apparently the lockfile library handles that itself.

sidharthachatterjee · 2020-08-04T08:05:29Z

@ascorbic Okay, that’s great then.

ascorbic · 2020-08-04T08:08:03Z

@sidharthachatterjee Added the log

Store pid Allow lockfile to be ignored for services that don't need to be running Get correct port for running service Add last run time init site info in gatsby new Store metadata on build Make getService generic so we can avoid casting Log exception

wardpeet

I've added some small cleanup tasks.

I only see you writing to the metadata file. Does gatsby desktop removes things from the metadata file? Mostly concerned about keeping pids inside the metadata when the process stopped.

wardpeet · 2020-08-05T07:37:46Z

packages/gatsby-cli/src/init-starter.ts

@@ -9,10 +9,11 @@ import isValid from "is-valid-path"
 import sysPath from "path"
 import prompts from "prompts"
 import url from "url"
-
+import { createServiceLock } from "gatsby-core-utils/dist/service-lock"


Suggested change

import { createServiceLock } from "gatsby-core-utils/dist/service-lock"

// TODO make service-lock export from gatsby-core-utils

import { createServiceLock } from "gatsby-core-utils/dist/service-lock"

This is because of this:

https://github.com/gatsbyjs/gatsby/blob/master/packages/gatsby-core-utils/src/service-lock.ts#L6

I see, thanks. I'll create a ticket for me to fix that

wardpeet · 2020-08-05T07:45:00Z

packages/gatsby-cli/src/init-starter.ts

+  const sitePath = sysPath.resolve(rootPath)
+
+  const sitePackageJson = await fs
+    .readJSON(sysPath.join(sitePath, `package.json`))
+    .catch(() => {
+      reporter.verbose(
+        `Could not read "${sysPath.join(sitePath, `package.json`)}"`
+      )
+    })
+
+  await createServiceLock(sitePath, `metadata`, {
+    name: sitePackageJson?.name || rootPath,
+    sitePath,
+    lastRun: Date.now(),
+  }).then(unlock => unlock?.())


Could we move this to a function so it's more clear what this does and why we need it? Maybe worth to move it into core-utils?

/** * the metadata file is used in gatsby desktop to discover which projects are created for gatsby */ function saveProjectToMetadataFile() {}

Yeah, though it would still need most of the same props. e.g. it won't always want to save the pid or update the lastRun (e.g. if it's being called from Desktop)

those values could be nullable/undefined maybe?

packages/gatsby-core-utils/src/service-lock.ts

wardpeet · 2020-08-05T07:56:38Z

packages/gatsby/src/commands/build.ts

@@ -36,6 +36,7 @@ import {
  markWebpackStatusAsPending,
  markWebpackStatusAsDone,
 } from "../utils/webpack-status"
+import { createServiceLock } from "gatsby-core-utils/dist/service-lock"


Suggested change

import { createServiceLock } from "gatsby-core-utils/dist/service-lock"

// TODO make service-lock export from gatsby-core-utils

import { createServiceLock } from "gatsby-core-utils/dist/service-lock"

wardpeet · 2020-08-05T07:57:56Z

packages/gatsby/src/commands/build.ts

+  await createServiceLock(program.directory, `metadata`, {
+    name: program.sitePackageJson.name,
+    sitePath: program.directory,
+    lastRun: Date.now(),


It doesn't harm us to keep it consistent and always add a pid, right? Do we also want to keep track of which method it is? (build/develop)

Does it make sense to add a typescript type for it?

Moving it to core-utils would solve this as well.

wardpeet · 2020-08-05T07:58:15Z

packages/gatsby/src/commands/develop.ts

      console.error(
-        `Looks like develop for this site is already running. Try visiting http://localhost:8000/ maybe?`
+        `Looks like develop for this site is already running. Try visiting http://localhost:${port}/ maybe?`


wardpeet · 2020-08-05T07:59:17Z

packages/gatsby/src/commands/develop.ts

+import {
+  createServiceLock,
+  getService,
+} from "gatsby-core-utils/dist/service-lock"


Suggested change

import {

createServiceLock,

getService,

} from "gatsby-core-utils/dist/service-lock"

// TODO make service-lock export from gatsby-core-utils

import {

createServiceLock,

getService,

} from "gatsby-core-utils/dist/service-lock"

wardpeet

i'll approve, would be nice if you could look into moving things to core-utils in another PR

ascorbic · 2020-08-05T10:27:59Z

I just tried to push a commit that would do just that. Oops!

ascorbic · 2020-08-05T10:33:12Z

@wardpeet Here's the followup PR: #26237

Store pid Allow lockfile to be ignored for services that don't need to be running Get correct port for running service Add last run time init site info in gatsby new Store metadata on build Make getService generic so we can avoid casting Log exception Co-authored-by: gatsbybot <[email protected]>

ascorbic requested a review from mxstbr July 31, 2020 10:08

ascorbic requested a review from a team as a code owner July 31, 2020 10:08

gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Jul 31, 2020

ascorbic added status: needs core review Currently awaiting review from Core team member and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Jul 31, 2020

mxstbr reviewed Jul 31, 2020

View reviewed changes

ascorbic requested a review from mxstbr August 3, 2020 13:53

mxstbr previously approved these changes Aug 4, 2020

View reviewed changes

sidharthachatterjee reviewed Aug 4, 2020

View reviewed changes

packages/gatsby-cli/src/init-starter.ts Outdated Show resolved Hide resolved

sidharthachatterjee previously approved these changes Aug 4, 2020

View reviewed changes

ascorbic dismissed stale reviews from sidharthachatterjee and mxstbr via eb0a176 August 4, 2020 08:08

mxstbr previously approved these changes Aug 4, 2020

View reviewed changes

ascorbic added the bot: merge on green Gatsbot will merge these PRs automatically when all tests passes label Aug 4, 2020

Store site metadata

cfa0974

Store pid Allow lockfile to be ignored for services that don't need to be running Get correct port for running service Add last run time init site info in gatsby new Store metadata on build Make getService generic so we can avoid casting Log exception

ascorbic dismissed mxstbr’s stale review via cfa0974 August 4, 2020 11:36

ascorbic force-pushed the feat/site-metadata branch from b74d996 to cfa0974 Compare August 4, 2020 11:36

Merge remote-tracking branch 'upstream/master' into feat/site-metadata

c8cdb12

wardpeet reviewed Aug 5, 2020

View reviewed changes

wardpeet approved these changes Aug 5, 2020

View reviewed changes

gatsbybot merged commit 36367c4 into master Aug 5, 2020

delete-merged-branch bot deleted the feat/site-metadata branch August 5, 2020 09:57

ascorbic restored the feat/site-metadata branch August 5, 2020 10:28

	import { createServiceLock } from "gatsby-core-utils/dist/service-lock"
	// TODO make service-lock export from gatsby-core-utils
	import { createServiceLock } from "gatsby-core-utils/dist/service-lock"

feat(gatsby): Store site metadata #26162

feat(gatsby): Store site metadata #26162

Conversation

ascorbic commented Jul 31, 2020

gatsby-cloud bot commented Jul 31, 2020 • edited Loading

Gatsby Cloud Build Report

Build Details

Performance

Lighthouse report

gatsby-cloud bot commented Jul 31, 2020 • edited Loading

Gatsby Cloud Build Report

Build Details

Performance

Lighthouse report

gatsby-cloud-staging bot commented Jul 31, 2020 • edited Loading

Gatsby Cloud Build Report

Build Details

Performance

Lighthouse report

gatsby-cloud bot commented Jul 31, 2020 • edited Loading

Gatsby Cloud Build Report

Build Details

Performance

Lighthouse report

gatsby-cloud-staging bot commented Jul 31, 2020 • edited Loading

Gatsby Cloud Build Report

Build Details

Performance

Lighthouse report

gatsby-cloud bot commented Jul 31, 2020 • edited Loading

Gatsby Cloud Build Report

Build Details

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mxstbr Jul 31, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

KyleAMathews commented Jul 31, 2020

mxstbr commented Jul 31, 2020

ascorbic commented Aug 3, 2020

KyleAMathews commented Aug 3, 2020

ascorbic commented Aug 3, 2020

mxstbr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wardpeet Aug 5, 2020 • edited Loading

Choose a reason for hiding this comment

ascorbic Aug 5, 2020 • edited Loading

Choose a reason for hiding this comment

sidharthachatterjee left a comment

Choose a reason for hiding this comment

ascorbic commented Aug 4, 2020

sidharthachatterjee commented Aug 4, 2020

ascorbic commented Aug 4, 2020

wardpeet 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

wardpeet Aug 5, 2020 • edited Loading

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

wardpeet Aug 5, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wardpeet left a comment

Choose a reason for hiding this comment

ascorbic commented Aug 5, 2020

ascorbic commented Aug 5, 2020

gatsby-cloud bot commented Jul 31, 2020 •

edited

Loading

gatsby-cloud bot commented Jul 31, 2020 •

edited

Loading

gatsby-cloud-staging bot commented Jul 31, 2020 •

edited

Loading

gatsby-cloud bot commented Jul 31, 2020 •

edited

Loading

gatsby-cloud-staging bot commented Jul 31, 2020 •

edited

Loading

gatsby-cloud bot commented Jul 31, 2020 •

edited

Loading

mxstbr Jul 31, 2020 •

edited

Loading

wardpeet Aug 5, 2020 •

edited

Loading

ascorbic Aug 5, 2020 •

edited

Loading

wardpeet Aug 5, 2020 •

edited

Loading

wardpeet Aug 5, 2020 •

edited

Loading