preact/debug #633

NekR · 2017-04-08T11:49:52Z

preact/debug is here 🎉

Usage

To use preact/debug, you have to set process.env.NODE_ENV to 'development' for your bundler, e.g. for webpack:

new webpack.DefinePlugin({
  'process.env': {
    'NODE_ENV': JSON.stringify('development')
  }
})

Also, preact/debug imports preact/devtools automatically 👌

Initialization

Basic

require('preact/debug');

// or

import 'preact/debug';

Configurable

require('preact/debug')({
  stringRefs: false
});

// or

import preactDebug from 'preact/debug';

preactDebug({
  stringRefs: false
});

Config options

(all turned on by default)

undefinedComponents: boolean, throw or not on undefined components
stringRefs: boolean, throw or not on string refs (this throws anyway, but better track trace with the option)
arrayChildren: boolean, warn or not about missing key properties on elements in array children

Warning example:

Side notes

Source directory is debug_src/ and is transpiled to debug/. Ideally source directory should be debug/ and transpiled to debug.js, but I don't know anyway to do so with requiring only './debug.js' module in it an ignoring everything else (can be done with browserify but WAT 😨).
Another way might be to publish separate package called preact-debug, but there is a problem that webpack makes ES6+ imports getter only so you won't be able to override preact.h property. If this could be worked around, then probably is the way to go.
preact.createElement isn't hooked just yet

NekR · 2017-04-08T12:14:36Z

If this landed, #601 might not be necessary (which was fixing #431).

NekR · 2017-04-08T14:28:01Z

About folder structure. Also if we are going preact-debug package way, we can make it export preact itself (this way we solve webpack imports problem) and it could be aliased in dev mode. Though it's easier to include debug in the code rather than in config :-)

lukeed · 2017-04-08T14:28:16Z

The debug_src can be dropped & everything moved to a single file (debug.js) if everything were wrapped within the NODE_ENV !== 'development' check.

robertknight · 2017-04-08T14:58:22Z

What is the rationale for making each of these warnings configurable? I would be inclined to make the debug package zero or minimally configurable until we have a clear use case, since that reduces the risk of having to make a breaking change in future.

robertknight · 2017-04-08T15:04:39Z

debug_src/debug.js

+	arrayChildren: true
+};
+
+preact.h = function(nodeName, attributes) {


Could this be built on Preact's existing options.vnode hook rather than patching h directly? Then it should work for createElement as well. This would also be consistent with how the devtools module works - hooking into Preact's global hooks.

It could, but that way you won't get good stack trace, i.e. the call before throwing the error will point directly to the place of h() invocation. If this won't provide good stack traces I'm not really interested in having this package myself.

I want to check if we can prune the stack trace - we'd know it's 2 levels up, and the stack is just a String so we could remove the first two lines. My reasoning is that preact is actually a frozen object when bundled via Webpack 2 - this is already causing issues with React Ho Loader since it does the patching technique used here, and I'm worried it's going to become an issue. Thoughts?

Stacks aren't that simple. Firefox have weird stack format and you cannot just modify it. There is no docs about the format and people in Mozilla IRC doesn't answer really to that question. There might be some lib by Fx extensions team, but I don't know about it.

It could, but that way you won't get good stack trace

Not sure I follow. Surely throwing in the vnode hook would just result in a trace with the point of the h(...) invocation being one more entry up in the stack trace?

robertknight

Couple of initial thoughts:

Could this be built on the existing global hooks rather than trying to replace exports directly? This would be consistent with how the devtools integration and a number of other debugging/testing aids for Preact work.
I would be inclined to drop the configurability in the initial release of this unless you have a clear use case for it.

NekR · 2017-04-08T15:07:44Z

@robertknight I for example need only undefinedComponents option, all other are annoying to me. Maybe stringRefs are fine, but arrayChildren for sure would annoy me.

NekR · 2017-04-08T15:10:21Z

@robertknight

I would be inclined to drop the configurability in the initial release of this unless you have a clear use case for it.

@developit suggests to not use forEach((a, i) => <div key={i} />) because it already use indexes internally and specifying them that way may more hurt than help. In that case, I would expect people to disable that warning.

This is in addition to that this option is just annoying for myself.

developit · 2017-04-08T15:48:35Z

I'm inclined to agree about the keys option, only because I've got codebases that would probably crash the console from the sheer number of warnings that would generate 😇

@NekR one option would be to hold off on releasing the key check since that's a pretty huge assertion on our part and a common cause of FUD around how keys work.

So maybe if we turned off key warnings for now we could release with no options? Still keep it a constructor but have the defaults be to warn both for undefined components and string refs.

BTW - I wonder if we need to detect preact-compat and disable the string ref warning if it's in-place, since that adds support for them?

NekR · 2017-04-08T16:15:34Z

till keep it a constructor but have the defaults be to warn both for undefined components and string refs.

As I said in PR comment all debug checks are ON by default :-) Config is there just to disable things.

BTW - I wonder if we need to detect preact-compat and disable the string ref warning if it's in-place, since that adds support for them?

Makes sense, not sure how to so it though.

developit · 2017-04-08T16:42:35Z

I just know the keys warning is something I've complained about in React a lot, I'm kinda tempted to avoid promoting that people go crazy with keys out-of-the-box haha.

NekR · 2017-04-08T18:40:53Z

@lukeed, @robertknight, @developit fine, fine, I'll do as you all want 😇

NekR · 2017-04-11T15:50:37Z

Okay, I did requested changed but didn't test them :-)

developit · 2017-04-15T01:28:41Z

debug.js

@@ -0,0 +1,90 @@
+'use strict';


we should .gitignore the built files here

developit · 2017-04-15T01:30:08Z

debug/index.js

+		return oldVnodeOption.call(this, vnode);
+	};
+
+	const inspectChildren = (children, inspect) => {


children should always be pre-flattened (though I'm fine leaving this here)

I'll remove that eventually, too lazy right now.

developit

Everything looks good, just thinking we'll want to gitignore the built files. Also it might be nice to bundle to UMD (-f umd) so they run in a browser context too?

NekR · 2017-04-15T01:59:55Z

@developit I did with debug the same what you do with devtools. Also, there is require('preact') in debug, how would that work in browser?

developit · 2017-04-15T02:14:12Z

Devtools seems to be UMD - https://unpkg.com/preact@8.1.0/devtools.js

The preact import gets transformed to a detection, should work nicely.

NekR · 2017-04-15T02:19:18Z

I mean with folder/generation. You have devtools/index.js folder and devtools.js file.

developit · 2017-04-16T02:33:10Z

True, and that's working (guess that's my answer haha).

developit · 2017-04-18T12:35:19Z

I think we're ready to merge this :)

NekR · 2017-04-18T12:55:45Z

@developit yeah, I think so. But if you wish it to be UMD then I can try that.

coveralls · 2017-04-22T05:58:03Z

Coverage remained the same at 100.0% when pulling eef7d70 on OpenJSX:master into 4a9aefc on developit:master.

coveralls · 2017-05-27T18:35:28Z

Changes Unknown when pulling b0263af on OpenJSX:master into ** on developit:master**.

NekR · 2017-05-27T18:36:05Z

I merged it ¯\_(ツ)_/¯

trueadm · 2017-05-28T19:11:11Z

In React, we make warnings only fire once so the console isn't spammed. Might be worth doing it here too :)

NekR · 2017-05-28T20:05:40Z

For the same element/component or in general write to console once for a each warning?

trueadm · 2017-05-28T20:24:37Z

For each warning usually. Spamming a console is known to have an adverse affect on DX. We had really great feedback when we changed how this worked.

developit · 2017-05-29T00:20:06Z

I like that idea. Keeping it global keeps things simple.

NekR · 2017-05-30T02:05:58Z

I have nothing against it, but I won't have time for it for some time. Someone may want to create a new issue for it and assign it to me :-)

…

On May 29, 2017 3:20 AM, "Jason Miller" ***@***.***> wrote: I like that idea. Keeping it global keeps things simple. — You are receiving this because you modified the open/close state. Reply to this email directly, view it on GitHub <#633 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ABIlkRIgExOFFMma8UEaOiP2V246eISbks5r-g84gaJpZM4M3rkj> .

rupav · 2018-05-30T17:32:52Z

@NekR I see a different and strange behavior of React-dev-tools using Preact/debug. React-dev-tools are active with this config only (as suggested in this PR description), in our project, we are using Preact with React as an alias.
I also would like to know if Redux-store dev tools can be configured for Preact? 😅

marvinhagemeister · 2018-05-30T19:21:18Z

@rupav the redux devtools should work right out of the box as they are not framework dependent.

rupav · 2018-05-31T03:11:12Z

Forgive me @marvinhagemeister for asking without checking again. I had not configured setup redux-dev-tools on configureStore. Yeah its working great. But what is your opinion on React-dev-tools with Preact/debug? How it can be best utilized.

marvinhagemeister · 2018-05-31T09:30:08Z

@rupav easiest way to do it is by including preact/debug. If something isn't working correctly you should file a bug :)

NekR added 2 commits April 8, 2017 00:50

preact/debug

086484c

preact/debug linting

50b0696

NekR requested review from developit and robertknight April 8, 2017 11:49

robertknight reviewed Apr 8, 2017

View reviewed changes

Merge branch 'master' into master

0bc4d88

jmsaulnier mentioned this pull request Apr 10, 2017

Roadmap 1.0.0 thinkingboxmedia/static-web-starter-kit#9

Open

21 tasks

preact/debug rewrite

397c5cc

NekR added 2 commits April 11, 2017 18:50

Merge branch 'master' into master

5361c42

preact/debug fix code formatting

3e6532c

NekR mentioned this pull request Apr 12, 2017

Preact 8.0.1 - Rendering Lists with duplicate key values causes "ghost" elements #642

Closed

NekR added 3 commits April 15, 2017 02:30

Merge branch 'master' into master

913430f

preact/debug report error on duplicate keys

7956ee3

preact/debug report error on duplicate keys

cb9e267

developit reviewed Apr 15, 2017

View reviewed changes

debug.js Outdated

@@ -0,0 +1,90 @@

'use strict';

Copy link

Member

developit Apr 15, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should .gitignore the built files here

developit reviewed Apr 15, 2017

View reviewed changes

developit approved these changes Apr 15, 2017

View reviewed changes

Merge branch 'master' into master

69e020a

NekR added 3 commits April 22, 2017 08:51

Merge branch 'master' into master

2218eb8

preact/debug remove build artefacts

73a6526

preact/debug ignore debug build artefacts

eef7d70

developit mentioned this pull request Apr 22, 2017

React for dev, Preact for prod ? #662

Closed

Merge branch 'master' into master

b0263af

NekR merged commit b80bec3 into preactjs:master May 27, 2017

ansballard mentioned this pull request Sep 7, 2017

Docs for require("preact/debug");? #858

Closed

rupav mentioned this pull request May 29, 2018

Add Preact/React developer tools in debug mode HelikarLab/candis#69

Merged

Uh oh!

preact/debug #633

preact/debug #633

Uh oh!

Conversation

NekR commented Apr 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Usage

Initialization

Config options

Side notes

Uh oh!

NekR commented Apr 8, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

NekR commented Apr 8, 2017

Uh oh!

lukeed commented Apr 8, 2017

Uh oh!

robertknight commented Apr 8, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

robertknight left a comment

Choose a reason for hiding this comment

Uh oh!

NekR commented Apr 8, 2017

Uh oh!

NekR commented Apr 8, 2017

Uh oh!

developit commented Apr 8, 2017

Uh oh!

NekR commented Apr 8, 2017

Uh oh!

developit commented Apr 8, 2017

Uh oh!

NekR commented Apr 8, 2017

Uh oh!

NekR commented Apr 11, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

developit left a comment

Choose a reason for hiding this comment

Uh oh!

NekR commented Apr 15, 2017

Uh oh!

developit commented Apr 15, 2017

Uh oh!

NekR commented Apr 15, 2017

Uh oh!

developit commented Apr 16, 2017

Uh oh!

developit commented Apr 18, 2017

Uh oh!

NekR commented Apr 18, 2017

Uh oh!

coveralls commented Apr 22, 2017

Uh oh!

coveralls commented May 27, 2017

Uh oh!

NekR commented May 27, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

trueadm commented May 28, 2017

Uh oh!

NekR commented May 28, 2017

Uh oh!

NekR commented Apr 8, 2017 •

edited

Loading

NekR commented Apr 8, 2017 •

edited

Loading

NekR commented May 27, 2017 •

edited

Loading

rupav commented May 31, 2018 •

edited

Loading