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

[Request] Expose Message.details formatter #1058

Closed
lukeed opened this issue Mar 25, 2021 · 11 comments
Closed

[Request] Expose Message.details formatter #1058

lukeed opened this issue Mar 25, 2021 · 11 comments

Comments

@lukeed
Copy link
Contributor

lukeed commented Mar 25, 2021

There's quite a bit of logic going into logging the messages, specifically the details.

Would it be possible to expose this function on the Message interface itself? Perhaps it only appears when logLevel is silent.

Specifically, I'd like to customize the "text" portion of each message & then use esbuild's formatted details.

Current
Screen Shot 2021-03-25 at 11 42 41 AM

Customized
Screen Shot 2021-03-25 at 11 42 48 AM

AFAIK, there's no way to catch-n-rewrite the stdout since this is build & esbuild writes it directly.
I can (and was) reimplement the output in the JS wrapper layer, but this feels a little silly given that the code is already included.

Perhaps the usage could look something like this:

// errors (or warnings)
let output = `Build failed with ${err.errors.length} error(s):`;
err.errors.forEach(msg => {
  output += '\n  ' + colors.inverse(msg.location.file) + ' ' + msg.text;
  output += '\n  ' + msg.details(); // or msg.frame() --> esbuild's code formatter
});
console.error(output);
@zaydek
Copy link

zaydek commented Mar 25, 2021

@lukeed Check this out #920. May help answer some of your questions.

You should be able to disable logs with logLevel: "silent" and then inspect result.warnings. I’m doing something like this currently, although with my new approach I don’t think I need to anymore:

const response = {
  warnings: [],
  errors: [],
}

try {
  result = await esbuild.build({
    // ...
  })
  if (result?.warnings?.length > 0) {
    response.warnings = result.warnings
  }
} catch (error) {
  if (error?.errors?.length > 0) {
    response.errors = error.errors
  }
  if (error?.warnings?.length > 0) {
    response.warnings = error.warnings
  }
}

// ...

@lukeed
Copy link
Contributor Author

lukeed commented Mar 25, 2021

@zaydek thank you -- I'm aware I can do this. I'm in agreement with the other thread. My point here is that because esbuild's formatting is so good & covers so many edge cases, that I don't even want to attempt to reimplement the same coverage in my JS layer. Instead, I'm hoping esbuild's formatting is available as a new method on the Message object.

@evanw
Copy link
Owner

evanw commented Mar 25, 2021

I think the way to solve this would be to expose a top-level API to format a message object. I want to keep message objects serializable so I don't want to put a method on the messages themselves. And splitting off the detail part seems unnecessary when you can easily just remove the first line.

@lukeed
Copy link
Contributor Author

lukeed commented Mar 25, 2021

Sounds good to me!

@evanw evanw closed this as completed in 5cf85d5 Mar 25, 2021
@lukeed
Copy link
Contributor Author

lukeed commented Mar 25, 2021

Amazing 🙌 thanks!

@zaydek
Copy link

zaydek commented Mar 26, 2021

Nice work Evan. This actually solves a problem I didn’t know I had. I can defer to esbuild now for logging aesthetics, which is amazing. This essentially perfectly solves for #920.

@lukeed Does this solve your problem? This look almost like a solution to a different problem if I’m not mistaken -- I thought what you were asking for is the ability to customize message UI before logging it. If this does solve your problem, do you mind sharing an example of how you’re using this feature?

@lukeed
Copy link
Contributor Author

lukeed commented Mar 26, 2021

Nope, this is exactly what I was after. I wanted to access esbuild's existing formatting so that I could call it programmatically.

I was hesitant to suggest a top-level API change, but this is definitely the better option.

@zaydek
Copy link

zaydek commented Mar 26, 2021

If you don’t mind, could you show how you plan on using the API with your original example?

// errors (or warnings)
let output = `Build failed with ${err.errors.length} error(s):`;
err.errors.forEach(msg => {
  output += '\n  ' + colors.inverse(msg.location.file) + ' ' + msg.text;
  output += '\n  ' + msg.details(); // or msg.frame() --> esbuild's code formatter
});
console.error(output);

Edit: Looking at the test cases I think I understand. Do you plan on calling esbuild.formatMessages multiple times per your customized message? That way you can customize only the bits you care about and ignore / keep the rest?

@lukeed
Copy link
Contributor Author

lukeed commented Mar 26, 2021

@zaydek No problem. Yes, multiple times or once with all messages & then iterate thru the returned array:

// errors (or warnings)
let output = `Build failed with ${err.errors.length} error(s):`;
let formatted = await esbuild.formatMessages(err.errors, { kind: 'error' });

formatted.forEach(str => {
  // optionally modify the output esbuild gave me
  // for example, remove the 1st line.. whatever
  
  output += '\n  ' + str;
});
console.error(output);

lukeed added a commit to lukeed/cfw that referenced this issue Mar 27, 2021
@zaydek
Copy link

zaydek commented Mar 27, 2021

@lukeed Thank you! Nice work to you and Evan in getting this implemented so quickly. Am using it myself already.

@lukeed
Copy link
Contributor Author

lukeed commented Mar 27, 2021

I contributed nothing 😂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants