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

Summory option in .build() #704

Closed
rogemus opened this issue Jan 22, 2021 · 5 comments
Closed

Summory option in .build() #704

rogemus opened this issue Jan 22, 2021 · 5 comments

Comments

@rogemus
Copy link

rogemus commented Jan 22, 2021

Hi, I wanted to create multiple scripts for esbuild so it easier to maintain configuration instead of strings in package.json.

Example:

const esbuild = require('esbuild')

esbuild
	.build({
		entryPoints: ['app/src/packages/login/index.tsx'],
		bundle: true,
		define: {
			'process.env.NODE_ENV': '\"production\"'
		},
		loader: { 
			'.svg': 'file',
			'.otf': 'file'
		},
		summary: true,
		outdir: 'login',

	})

Sadly when I use the summary feature in config I receive an error in the terminal:

error: Invalid option in build() call: "summary"
    230 │       throw new Error(`Invalid option ${where}: "${key}"`);

Is --summary only available in CLI?

@evanw
Copy link
Owner

evanw commented Jan 25, 2021

Is --summary only available in CLI?

Yes. It's an experiment so I only added it to the CLI. I'm considering just making this the default behavior in all cases and removing the --summary option instead of adding it to the other API surfaces. The way to turn it off would then be to use --log-level=warning or higher (it would turn on with --log-level=info).

@rogemus
Copy link
Author

rogemus commented Jan 27, 2021

Ok thx for answer :). I will just wait some time for this to become the default behaviour ;D

@evanw evanw mentioned this issue Mar 6, 2021
10 tasks
@zaydek
Copy link

zaydek commented Mar 6, 2021

I wanted to get a sense of what the summary options feels like so I took a screenshot for myself and or others:

Screen Shot 2021-03-06 at 6 57 53 PM

My only concern is that when using esbuild with yarn for example, is that the output is going to get very noisy since yarn also decorates outputs with its own Done in X seconds footer. I’m not sure it actually makes sense for esbuild output to be shaped by yarn, but I felt I should bring attention to this.

Given the you have outdir and outbase options, would it make more sense to put milliseconds next to every file instead of as a footer? That way users can see, whether for one file or for many, how long each file took to process. I think putting millisecond annotations to the right of outfile or outdir may play better with yarn.

Millisecond annotations per-file may not make perfect sense with how esbuild works, given it uses concurrency, etc., but you can also measure the differences between processing file A from file B.

In my own project (which uses esbuild), output looks like this. Maybe there is some inspiration to be shared?

Implementation here:
import * as esbuild from "esbuild"
import * as path from "path"
import * as terminal from "../shared/terminal"
import * as types from "./types"
import * as utils from "./utils"

const TERM_WIDTH = 40

function formatMS(ms: number): string {
  switch (true) {
    case ms < 250:
      // 250ms
      return `${ms}ms`
    default:
      // 0.25ms
      return `${(ms / 1e3).toFixed(2)}s`
  }
}

export function export_(runtime: types.Runtime, meta: types.RouteMeta, start: number): void {
  const dur = formatMS(Date.now() - start)

  const l1 = runtime.directories.srcPagesDirectory.length
  const l2 = runtime.directories.exportDirectory.length

  let color = terminal.white
  if (meta.routeInfo.type === "dynamic") {
    color = terminal.cyan
  }

  let dimColor = terminal.dim.white
  if (meta.routeInfo.type === "dynamic") {
    dimColor = terminal.dim.cyan
  }

  const src = meta.routeInfo.src.slice(l1)
  const src_ext = path.extname(src)
  const src_name = src.slice(1, -src_ext.length)

  const dst = meta.routeInfo.dst.slice(l2)
  const dst_ext = path.extname(dst)
  const dst_name = dst.slice(1, -dst_ext.length)

  const sep = "-".repeat(Math.max(0, TERM_WIDTH - `/${src_name}${src_ext}\x20`.length))

  console.log(
    `\x20${terminal.dim(utils.timestamp())}\x20\x20` +
      `${dimColor("/")}${color(src_name)}${dimColor(src_ext)} ${dimColor(sep)} ${dimColor("/")}${color(dst_name)}${
        start === 0 ? "" : ` ${dimColor(`(${dur})`)}`
      }`,
  )
}

export function serve(args: esbuild.ServeOnRequestArgs): void {
  const dur = formatMS(args.timeInMS)

  let color = terminal.normal
  if (args.status < 200 || args.status >= 300) {
    color = terminal.red
  }

  let dimColor = terminal.dim
  if (args.status < 200 || args.status >= 300) {
    dimColor = terminal.dim.red
  }

  let logger = (...args: unknown[]): void => console.log(...args)
  if (args.status < 200 || args.status >= 300) {
    logger = (...args) => console.error(...args) // eslint-disable-line
  }

  const path_ = args.path
  const path_ext = path.extname(path_)
  const path_name = path_.slice(1, -path_ext.length)

  const sep = "-".repeat(Math.max(0, TERM_WIDTH - `/${path_name}${path_ext}\x20`.length))

  logger(
    `\x20${terminal.dim(utils.timestamp())}\x20\x20` +
      `${dimColor("/")}${color(path_name)}${dimColor(path_ext)} ${dimColor(sep)} ${color(args.status)} ${dimColor(
        `(${dur})`,
      )}`,
  )
}

I recorded my output to provide an example. Also a screenshot since I think GH requires the video be downloaded first? Clicking this apparently downloads the video.

Screen Shot 2021-03-06 at 7 16 21 PM

retro-1080.mov

Edit: I now see that --summary has been supported since 0.8.28 so you probably won’t want to change it. 😅

@evanw
Copy link
Owner

evanw commented Mar 6, 2021

Edit: I now see that --summary has been supported since 0.8.28 so you probably won’t want to change it. 😅

I'm fine with changing it. People shouldn't be parsing it (e.g. it's truncated over a certain number of entries) so it should only be for humans.

My only concern is that when using esbuild with yarn for example, is that the output is going to get very noisy since yarn also decorates outputs with its own Done in X seconds footer. I’m not sure it actually makes sense for esbuild output to be shaped by yarn, but I felt I should bring attention to this.

Good to know. Thanks for calling this out. As you can tell I don't use yarn myself. It looks like this only happens with yarn 1 but not with yarn 2? Technically I could just not print the time if the npm_config_user_agent environment variable contains yarn/1.. Maybe I'll try that.

That way users can see, whether for one file or for many, how long each file took to process.

I don't think there is a number you can assign to each file that would mean something useful. Go has a preemptable task scheduler so just measuring the time difference between the start and the end of a task doesn't mean the task was running for that whole time. And also esbuild is heavily multithreaded so all of the times would add up to much longer than the actual running time, which is misleading IMO.

@zaydek
Copy link

zaydek commented Mar 6, 2021

Good to know. Thanks for calling this out. As you can tell I don't use yarn myself. It looks like this only happens with yarn 1 but not with yarn 2? Technically I could just not print the time if the npm_config_user_agent environment variable contains yarn/1.. Maybe I'll try that.

I’m on Yarn 1.22 and I always see this footer when running commands such as yarn esbuild. I don’t know whether Yarn 2 also does this, but I would expect 90%+ of your Yarn-based users to be on 1.x.

Yarn’s decorative text gets annoying when you are relying on terminal output for debugging, so I’ve started doing ./node_modules/.bin/esbuild and more recently I just alias esbuild as alias esbuild=node_modules/.bin/esbuild in my .bash_profile. I think there’s a silent flag for yarn but it’s tripped me up in the past so I don’t bother with it anymore.

Maybe I should just use npm run-script? 🤷‍♂️

General example:

Screen Shot 2021-03-06 at 9 29 36 PM

Yarn + esbuild example: (I’m on a Mac FWIW)

Screen Shot 2021-03-06 at 9 30 14 PM

I don't think there is a number you can assign to each file that would mean something useful.

Anyway, I do think it’s important for your users to understand how fast esbuild is without the need for time esbuild ....

Somewhat related: Create React App does some interesting profiling when building (i.e. react-scripts build) where it compares your previous build against your current build (not time measured, but bundle sizes). I found this image online so you can see what I mean. This might be a nice enhancement to summary output, reporting time measured and bundle size deltas. (Just an idea, not requesting this)

One interesting direction you could take this is emitting some kind of stats file (a little like your metafile idea, but for bundle statistics / performance metrics). One way I’ve done this in the past (not related to esbuild) is by running a small script on build so every release includes some metadata. If you were to support something like this for esbuild with normal builds (maybe a stats flag?) then esbuild would be able to compare stats from previous builds assuming it relates to the same build.

Sorry for getting off-topic. I just think terminal UIs are generally under-appreciated but is actually critical to adopting new users.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment