Skip to content

$inspect(...).with(...) #9737

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

Closed
Rich-Harris opened this issue Dec 1, 2023 · 3 comments · Fixed by #9838
Closed

$inspect(...).with(...) #9737

Rich-Harris opened this issue Dec 1, 2023 · 3 comments · Fixed by #9838
Assignees

Comments

@Rich-Harris
Copy link
Member

Describe the problem

Some feedback we've had on $inspect is that this...

console.log('>>> HERE', foo, bar, baz);

...is a common pattern that doesn't work with $inspect, as it only takes a single value argument (in order to prevent ambiguity when providing a callback function).

Describe the proposed solution

@dummdidumm had a great idea — allow an arbitrary number of arguments, and put the callback in a .with method:

// these are effectively identical
$inspect('>>> HERE', foo, bar, baz);
$inspect('>>> HERE', foo, bar, baz).with(console.log);

// can also use `console.trace` or `debugger`...
$inspect('>>> HERE', foo, bar, baz).with(console.trace);

$inspect('>>> HERE', foo, bar, baz).with((type, ...values) => {
  debugger;
});

// ...or whatever custom logic you want to provide
$inspect('>>> HERE', foo, bar, baz).with((type, ...values) => {
  klaxon.play();
});

Alternatives considered

n/a

Importance

nice to have

@Rich-Harris Rich-Harris mentioned this issue Dec 1, 2023
14 tasks
@dooshiifox
Copy link

dooshiifox commented Dec 4, 2023

I don't think the way this feature is currently planned is great from a developer point-of-view. As a user of Svelte, I understand that these are macros and so can do whatever the hell they want, but they still look and feel like they should abide by how JavaScript works. As such, having the functionality of $inspect change depending on whether theres a method on the end feels... wrong. Perhaps something like $inspect.many(['>>> HERE', foo, bar, baz], (type, ...values) => { debugger; }) and keeping the current behaviour of $inspect?
That being said, the idea is useful, and I'll be happy to see it implemented either way.

@Not-Jayden
Copy link
Contributor

Not-Jayden commented Dec 4, 2023

// these are effectively identical
$inspect('>>> HERE', foo, bar, baz);
$inspect('>>> HERE', foo, bar, baz).with(console.log);

Can you clarify with these two cases. Is the expectation that they would actually be identical, or would the first one omit the type that's included in the .with() callback?
i.e.

$inspect('>>> HERE', foo, bar, baz); // Output: '>>> HERE foo bar baz'
$inspect('>>> HERE', foo, bar, baz).with(console.log); // Output: 'init >>> HERE foo bar baz'

That would be my expected/preferred behaviour, but maybe that could be considered confusing.

@Not-Jayden
Copy link
Contributor

Not-Jayden commented Dec 4, 2023

Perhaps .with() could have an optional includeType second config argument.

e.g.

$inspect('>>> HERE', foo, bar, baz).with(console.log, true); // Output: 'init >>> HERE foo bar baz'
$inspect('>>> HERE', foo, bar, baz).with(console.log, false); // Output: '>>> HERE foo bar baz'

Whether it should default to true or false I'm not too sure, though I'd probably lean towards false to make the example mentioned actually identical.

Also questionable if it should be a config object rather than a simple boolean for easier extensibility in the future 🤷‍♂️

Typescript example

@dummdidumm dummdidumm self-assigned this Dec 7, 2023
dummdidumm added a commit that referenced this issue Dec 7, 2023
`$inspect` now takes 1-n arguments, and inspections modification happens through `.with(..)`
closes #9737
Rich-Harris pushed a commit that referenced this issue Dec 7, 2023
* breaking: change `$inspect` API

`$inspect` now takes 1-n arguments, and inspections modification happens through `.with(..)`
closes #9737

* lint
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.

4 participants