RFC: createElement changes and surrounding deprecations

This proposal simplifies how React.createElement works and ultimately lets us remove the need for forwardRef.

Deprecate "module pattern" components.
Deprecate defaultProps on function components.
Deprecate spreading key from objects.
Deprecate string refs (and remove production mode _owner field).
Move ref extraction to class render time and forwardRef render time.
Move defaultProps resolution to class render time.
Change JSX transpilers to use a new element creation method.

Always pass children as props.
Pass key separately from other props.
In DEV,

Pass a flag determining if it was static or not.
Pass __source and __self separately from other props.

The goal is to make element creation as simple as:
function jsx(type, props, key) {
 return {
   $$typeof: ReactElementSymbol,
   type,
   key,
   props,
 };
}

View Rendered Text

This awesome and actually not scary at all in my opinion :+1: Having to delete both key and ref from props was always a bit unfortunate. I'm wondering if the jsx constructor should accept ref as an argument too.
function jsx(type, props, key, ref) {
 return {
   $$typeof: ReactElementSymbol,
   type,
   key,
   ref,
   props,
 };
}

Pinging @DanielRosenwasser as this will have implications on the built-in jsx transpilation in TypeScript.

The original plan was to pass ref separately too but now I’m thinking that ref should just be a normal prop in all cases except when it is passed to a class component. In that case we can strip it off in a shallow clone of the props that excludes it.
The original reason for treating ref separately was because it easy to accidentally spread it along (and before that transferPropsTo).
The normal pattern for spread is that you should combine it with rest to pick off any properties you consume.
let [myCustomProp, ...domStuff] = props;
doSomething(myCustomProp);
return
;

The problem is that you never “do” anything with ref on a class component. It has already been automatically consumed in React. So if ref was part of props, then in this pattern it is easy to forget to avoid passing along the ref. Which is extra bad because refs really should only be attached to one thing at a time.
However if you want to attach a ref on a function component you do need to explicitly do something with the ref.
let [myCustomProp, ref, ...domStuff] = props;
doSomething(myCustomProp);
useImperativeHandle(ref, ...);
return
;

So the reason for special casing ref no longer exists in the Hooks world. In fact, the reason it is special cases makes it worse in the Hooks world because you need to use forwardRef instead of just a plain function component.
Therefore I think the right call here is to stop special casing it at the element level and instead start special casing it only for classes.

@sebmarkbage Ohh right, now I see where you're getting at. I didn't fully grok the part about forwardRef. Just using the ref prop directly instead of having to create an intermediate forwardRef component is a lot more elegant. This makes this proposal even more exciting :100:
Regarding keys: This proposal is mainly talking about createElement, but I'm wondering if the key extraction would affect cloneElement in the same way. They both share the same type-signature.

@uniqueiniquity @ryancavanaugh @weswigham

This is cool, thank you!

So for every set of props, we have to do an expensive dynamic property check to see if there is a key prop in there.

I'm a bit confused why this is expensive? Seems we just do the check in DEV and even if in DEV it just call hasOwnProperty which can't be expensive

To minimize churn and open up a larger discussion about this syntax

I don't think this is a good idea/start for React, we don't introduce any syntax like this('@'), all syntaxes we import is normal javascript syntax, but '@' is not(except decorator but that situation is not appropriate for this)
I would suggest that just make this become a breaking change because this spread usage is not popular(even is just edge case) from my perspective

Change JSX transpilers to use a new element creation method. Always pass children as props.

This will probably break other libraries that use JSX(unless it's configurable) and would still require a poly/megamorphic access for the runtime to access children props for host elements. It might be safer to define a strict arity(3) for createElement(type: any, props: object, children: any[]).
A best of both worlds will be for this to only apply to <Component> nodes, i.e <Component>1<Component> would emit createElement(Component, {children: 1}, null) and
1
 would emit createElement('h1', {}, [1]) this will avoid arbitrary arity and avoid potentially megamorphic access on the props object by the runtime.

I feel like this would be the perfect opportunity to introduce flags to the ‘jsx’ function call signature too. It would mean altering this RFC slightly, but just like how Inferno, Ivi and Vue 3 do, they use the concept of bitwise flags to contain information about the virtual node. For example, the flags could easily tell the reconciler if there are keyed children, or if the props are static, avoiding unnecessary diffs and further improving update performance. This would obviously require the JSX compiler to insert this information, but it’s proven highly beneficial to other libraries.

@trueadm That's a great point 👍 One reason we held off on doing this for preact is the various different forms of jsx transpilation. We have quite a few users who just use plain TS and don't use babel.
If the flags idea would proceed, we'd need to agree and standardise on a common set of values. There is obviously the risk of tying it too much to one particular jsx implementation. Looking at the flags in inferno there are several ones that are very specific to them. Same for vue.
Nonetheless it is a very interesting idea and I'd love to see it pursued further 👍

Huge +1 to adding flags, but with extreme caution: it should be a bitmask of values that indicated JSX source assumptions, not library optimizations - that's the only way this could work across frameworks. It could start simple - static props, static subtree, static children, is element, is component. All of these have unfortunate bailouts when spread or direct jsx() calls are used, so we can only ever hope to use them as informative.

@marvinhagemeister I'm not sue this would need to be standardized - there are no changes to the JSX spec. Furthermore, I don't see how this proposal would affect other libraries - unless they use babel-plugin-react, which they probably shouldn't be.
I guess Preact should use babel-plugin-preact or something along those lines (maybe it already does?), in which case this RFC should have no side-effects on other projects. Maybe I'm missing something here too? I know there might be issues with cross-compat support using this API, but I was under the assumption that Inferno and Preact had moved away from trying to be 100% compatible because of things like concurrent rendering and suspense making it almost impossible.

Huge +1 to adding flags, but with extreme caution: it should be a bitmask of values that indicated JSX source assumptions, not library optimizations - that's the only way this could work across frameworks. It could start simple - static props, static subtree, static children, is element, is component. All of these have unfortunate bailouts when spread or direct jsx() calls are used, so we can only ever hope to use them as informative.

I'd be happy with this too. Would make a lot of sense and I guess this is what @marvinhagemeister meant originally. Although, I'm still unsure of why these need to be compatible in the plugin level? Aren't libraries using their own Babel/TS transforms these days?

I don't see how this proposal would affect other libraries - unless they use babel-plugin-react

@trueadm They do use the pragma option provided: https://babeljs.io/docs/en/babel-plugin-transform-react-jsx and this is not uncommon/taboo since it is ofetn easier to use the pragma configuration of the tools default plugin than to integrate a custom third-party plugin.
{
  "plugins": [
    ["@babel/plugin-transform-react-jsx", {
      "pragma": "dom", // default pragma is React.createElement
      "pragmaFrag": "DomFrag", // default is React.Fragment
      "throwIfNamespace": false // defaults to true
    }]
  ]
}

My only other suggestion with regards to the JSX to JS generation is that this should aim to make iterating and normalising over static children obsolete.
That is

Hello
{[name, null, 1]}
. Should produce something akin to:
jsx.node('h1', null, jsx.children([
    jsx.node('p', null, jsx.text('Hello')),
    jsx.from([name, null, 1])
]))

function node (type, props, children) { return {type, props, children} }
function children (arr) {return arr}
function text (val) { return val }
function from (val) { /* check typeof dynamic value */ }

@thysultan I was under the impression that pragma would still work as it does now, except it would use the current createElement signature? You would just reference babel-plugin-legacy-react or something.
Your idea is quite neat, although it also sounds like you’ll be creating far more virtual objects than we do now?

@Jessidhia this might be a great time for us to actually perform a major breaking change on React types, since this overturns quite a lot of the assumptions around JSX type checking, and we'd need to change the signatures of React.createElement (well, straight up rename it to something else!)

although it also sounds like you’ll be creating far more virtual objects than we do now?

@trueadm The example doesn't create any more objects than what is currently done/what this proposal plans towards. It however does tries to ensure that:

Normalisation is isolated to explicit dynamic parts delegated/isolated to a single responsibility function i.e jsx.from/jsxFROM.
Arity is always maintained.
Single responsibility functions, text, empty, node, children that further isolate hot paths in case future internal library changes want to explore other internal data-structure layouts without breaking compact. Which is why the children, text functions in my example are identity functions that return their arguments as is – they could alternatively execute their own logic depending on what data-structure for these types fits the lib.
Monomorphic is maintained as much as possible outside of the dynamic parts.

If the transpiler is more aggressive it could also promote static-like arrays [name, null, 1] to being static.
jsx.node('h1', null, jsx.children([
-    jsx.node('p', null, jsx.text('Hello')),
+    jsx.node('p', null, jsx.text('Hello', -4294967295 /* index-hash-as-implicit-key */)),
-    jsx.from([name, null, 1])
+    jsx.children([jsx.from(name), jsx.empty(/* key */), jsx.text(1, /* key */)])
]))

+ function text(val, key) { /* ... */ }
+ function empty(key) { /* ... */ }

..and bail out as needed for more complex cases, it should at the very least be flexible enough to improve on such cases as needed, i.e It could probably play it safe and bail out of:

{data.map(v => v)}

jsx.node('h1', null, jsx.children([jsx.from(data.map(v => v))]))

or play it strict and...

{data.map(v => v)}

jsx.node('h1', null, jsx.children(data.map(v => jsx.from(v))))

The problem is that not all tooling supports adding new dependencies from a transform. The first step is figuring out how this can be done idiomatically in the current ecosystem.

Babel 7 definitely can, logan made helpers for it that are also capable of telling between ES or commonjs modules and injects import or require appropriately, and can be reused from any plugin.
This probably won't fly with people that still use babel-standalone in the browser for example but I wonder if that's used for anything beyond toy projects.

Not a big fan of @key, though other ideas that come to mind are either more confusing or would break all current jsx parsers. Probably the least bad one is <Component:constantKey /> / <Component:{expressionKey} /> but that sure looks like it could be confused with xmlns.

I'll probably make more comments after I've slept because it is 2am 😝

@thysultan so you’d use flags under the hood? If this isn’t going to be something you have write, then why have all these helper methods at all and skip the intermediate abstraction and just do a single call with flags passed in? It will result in less bytes in the app code and you will not need many function calls.
I’m not against your API, I just don’t see any advantages to it cause just inlining bitwise flags that contain more information, using less bytes.

@trueadm There are a few reasons.

Engines can better inline small single responsibility functions, they are effectively free except for the dynamic variant jsx.from which would need branching etc, at which point we are either invoking the same number of functions or less.
This in turn means it is less bytes, because there are less bit flags that are always being passed around, i.e the difference between(manually minified) n('h1', null, c([])) and j(1, 'h1', {children: []}, ...).
Less branching – with bit flags you would have to pass these(flags) to every callsite this would in turn mean that you would need to execute bitwise operations for every invocation and introduce branching into every call context, with my suggested case each function is a single responsibility citizen so there's less to no branching in the static case, which makes for a good direct threaded dispatch pipeline which is positive feedback for point 1. about making it easier for inlining heuristics.

At the root of it, my suggestion tips this on its head so that instead of assuming everything as dynamic and passing flags to signal static tree's assume everything is static and isolate dynamic trees, you can then pass as many flags as needed to only these dynamic parts or improve the transpiler incrementally to better understand the static parts within outwardly looking dynamic parts as demonstrated in the [name, null, 1] array example.
At the end of the day walking the whole tree is a walk in the park(pun intended) compared to the cost of generating these virtual snapshots, and i don't mean creating the end products(virtual objects), but the tangled soup of dealing with arbitrary arity, normalisation and the necessary branching need to archive these. So an attempt to avoid these should be at the helm of any optimisation centric approach.

@thysultan

Engines can better inline small single responsibility functions, they are effectively free except for the dynamic variant jsx.from which would need branching etc, at which point we are either invoking the same number of functions or less.

This isn't always the case and it's generally much better to use a single function. Essentially, it really depends on the JS engine and how much of exhaustive the inline cache is. You can easily show JITs performing well on these cases in microbenchmarks, but in real-world applications with several hundred kb of JS, you'll find the inline cache gets consumed very quickly.

2. This in turn means it is less bytes, because there are less bit flags that are always being passed around, i.e the difference between(manually minified) n('h1', null, c([])) and j(1, 'h1', {children: []}, ...).

That's true, but in reality you're trading a function call vs passing an inline object literal, which would be offset by gzip/brotli compression.

3. Less branching – with bit flags you would have to pass these(flags) to every callsite this would in turn mean that you would need to execute bitwise operations for every invocation and introduce branching into every call context, with my suggested case each function is a single responsibility citizen so there's less to no branching in the static case, which makes for a good direct threaded dispatch pipeline which is positive feedback for point 1. about making it easier for inlining heuristics.

Bit wise flags are extremely cheap to do and far more effective than object lookups, property lookups, switch statements. Plus they generally get optimized at a CPU level, because they're just arithmetic operations. I'm happy to dig more into this if you're interested in knowing more.

At the end of the day walking the whole tree is a walk in the park(pun intended) compared to the cost of generating these virtual snapshots, and i don't mean creating the end products(virtual objects), but the tangled soup of dealing with arbitrary arity, normalisation and the necessary branching need to archive these. So an attempt to avoid these should be at the helm of any optimisation centric approach.

I think we're aiming for the same thing here. The issue is that we have to be careful as to how we approach this now. We want to get this right and we want to enable future long-term compiler optimizations without over-complicating what we need in the short-term.
This idea really isn't about walking the tree faster, it's far simpler than that - it's how we encode additional information into React Element objects when compiled from JSX. I'm suggesting we add a "flags" field to represent this data, rather than having several objects or several fields that essentially do the same thing.
The next point, is why do we need this additional information? Well, at build time, we generally have lots of information that is never accessible at runtime. For example, if we have access to TypeScript or Flow annotations at build time, and that information is passed to a JSX element, we could use that information to infer that a component never re-renders or that the props of a component never change. By encoding this information into a field, we can substantially reduce the actual computation and memory usage at runtime.

Bit wise flags are extremely cheap to do and far more effective than object lookups.

When i mentiond jsx.node or jsx.text this is more a readable presentation of for example jsxNode or jsxText that do not involve object lookups.

The issue is that we have to be careful as to how we approach this now. We want to get this right and we want to enable future long-term compiler optimizations without over-complicating what we need in the short-term.

That's that i was trying to convey with the ethos of my suggestion, bitwise flags are by their nature less future-flexible you have to agree ahead of time. That said i'm not against flags, just that flags-if-any should be delegated exclusively to the dynamic parts and should not inherently pollute the pipeline of static representations, which is what jsx.from/jsFrom serve to represent in the examples i posted.
That is assume everything is static unless it's signal'ed as dynamic.

@thysultan Thanks for making that part clear. I understand you better now. I think one of the core parts of why I feel so strong for flags, is that it can remove all function calls entirely in production mode. Rather than using jsx(...) in production, the compiler could inline an array, with a dynamic signature that still remains fully monomorphic [flags, ...anyInputsDeterminedByFlags].
I've been using this approach for some experimentation with React compilation (full compilation, using type annotations) and it's extremely efficient both with a JIT and also highly efficient without one (important for low-end devices that don't use a JIT due to memory constraints). This is testing this approach on real-world apps with hundreds of thousands of objects are created, not just in synthetic benchmarks.

@trueadm Yes, the proposed monolith jsx() or granular text/node/empty/children/from could both serve as intermediate representations with a 2nd tier compiler that translates these further down the pipeline. I wonder if the 2nd tiers artefacts could be standardised as well given the divergence of implementation details between libraries.
On thing the current proposal will find hard to avoid with/without flags is the mega-morphic access that is props.children from the heuristic of implicitly assigning children to props for host elements(non-component types) instead of a strict Array type parameter...
Because at some point the runtime will need to retrieve props.children to extract children and the props shape is probably going to be all over the place with any real app. This ties into my previous suggestion.

@thysultan You'd possibly use a flag to indicate it is a host node:
[HOST_COMPONENT | HAS_STATIC_CHILD_NODE | HAS_STATIC_PROPS, { className: "foo" }, [TEXT_CHILD, "Hello world"]]
For example... but I'm not saying we have to go that far right now. We should start to think about that now though, so we don't make those compiler optimizations impossible in the future.

@Jessidhia we changed that for Preact X which is right around the corner. children won't always be an array anymore in Preact. We'll go into more detail on our true announcement once it's out 👍
@trueadm love the direction this is taking. I'm on board 👍

@Jessidhia

It'd also be nice do deal with the duality of children being sometimes a value, sometimes an array, but that'd require a React 18. Preact has children always as arrays.

I'd note that this wouldn't matter at all if children were completely opaque. Which they eventually might become. Traversing children breaks abstraction because usually it makes it impossible to add a component layer in the middle without breaking subtle assumptions. Of course, there are cases like Menu and MenuItem components that want to be aware of children. We think the solution to this isn’t easier introspection (which arrays don’t even help that much with — because those can be nested), but it’s a first-class multipass render API. That’s what Call/Return experiment tried to solve. We scrapped that API because it was confusing but the end game is the same.

I realized that the key can't really be pass through in the current semantics. It might have to be:
function jsx(type, props, key) {
 return {
   $$typeof: ReactElementSymbol,
   type,
   key: key === undefined ? null : '' + key,
   props,
 };
}

The semantics are quite weird because undefined turns into null but null actually turns into "null" because a defined value is stringified.
This exists mostly to ensure somewhat consistent type signatures of these fields.
We could special case defined vs not defined (like hasOwnProperty or in vs undefined) but the big question is whether a dynamic values such as
 is allowed to have undefined mean missing key.
It might make for a difficult upgrade path to change that given that we can't really know for sure on every key callsite whether it might be undefined. E.g.
. Stringifying these cases would cause a duplicate key very easily which would be bad.

Some feedback regarding defaultProps deprecation after migrating around 90 components away from defaultProps:

bundle size went up (+0.05% parsed, +0.50% gzipped).
IE11 and chrome 45 don't support object destructuring assigment which doesn't transpile well.
I would hope that this will be mitigated by react itself shipping less code to resolve defaultProps.
If a parent wants to read from the props of its children it will not read the default value. This is only possible with defaultProps.
current doc tooling (react-docgen; are there better alternatives?) requires either destructuring in function signature or custom handler
function and class components move farther apart: Before we had a single API for default values. Now we have to consider the component type.
Better typing experience. Latest type versions support defaultProps properly but there are still some edge cases when dealing with higher-order components.

@eps1lon This is great research and feedback!
Some follow up questions:

Do you happen to have a bundle comparison that doesn't transpile the destructuring assignment but still minifies? It would be nice to have a "modern JS" comparison since hopefully people will eventually be able to ship the untranspiled form.
Yea, this is the major limitation which will also apply to class components with this full proposal. The idea is to save as much work as possible at the actual callsites creating JSX. The common case is that the parent doesn't read. Longer term, we'd like to find alternatives so that it's not needed to read the props. That way they could be opaque and bound directly to the function that is going to be called. That way it can eagerly resolve slots instead of materializing a temporary object.
Can you elaborate on the types of patterns that you'd expect to be supported but aren't?

Can you elaborate on the types of patterns that you'd expect to be supported but aren't?

// A
// default value of `foo` recognized
function Component({ foo = 1, bar }) {
   //
}
// B
// function body is hidden
function Component(props) {
  const { foo = 1, bar } = props;
}

With more props or wrappers like memo or forwardRef pattern A becomes quite ugly. Purely stylistic though so this is at least for me less of an issue. It seems not too hard though for static analysis to find the props destructuring in the function body.

It would be nice to have a "modern JS" comparison since hopefully people will eventually be able to ship the untranspiled form.

I got confused with the chrome version. 41 is the version still relevant though this is more likely to improve in the foreseeable future. ~Leaves~ IE11 and we can only pray that usage drops more. Edit: I'm a bit confused by the behavior of babel-preset-env: I need to drop support for edge < 18, firefox < 53 and chrome < 51. This doesn't match the mdn page nor caniuse. Even the data source lists edge 17 as a browser support object destructuring. Looks like something is outdated.

Do you happen to have a bundle comparison that doesn't transpile the destructuring assignment but still minifies?

So with
// browserslistrc
edge >= 18
firefox >= 53
chrome >= 51
safari >= 10
node 8.0

We get (just want to emphasize that this is not just component code; there's a bit (20%?) noise from 3rd party libraries):

parsed
gzipped

before
294_238 B
80_369 B

after
290_300 B
79_655 B

absolute diff
-3_938 B
-714 B

relative diff
-1.3%
-0.8%

Which is somewhat expected since default props can't be mangled.

That way it can eagerly resolve slots instead of materializing a temporary object.

Love it. Didn't understand a word :smile:

If ref are going to be part of props, will it be considered by React.memo? If so, what about refs passed as inline functions?

I have something to add in defence of defaultProps for functional components
What if at destructuring props, you have a function or object as default value, and you are passing it down as prop to some child component? it will always be a new reference, hence the child component will be re-rendered all the time. it is pretty much catchy case, and the only solution I had is to use defaultProps or declare new const in the same file as the component, and use const as default value.

it will always be a new reference, hence the child component will be re-rendered all the time.

If you mean React.memo I don't think this is true, because props are not really modified on outside, but assigned to undefined values inside the component.

I have something to add in defence of defaultProps for functional components
What if at destructuring props, you have a function or object as default value, and you are passing it down as prop to some child component? it will always be a new reference, hence the child component will be re-rendered all the time. it is pretty much catchy case, and the only solution I had is to use defaultProps or declare new const in the same file as the component, and use const as default value.

You can just define the function/object outside of the destructuring assignment and reference it there. Same reference every time.

@JustFly1984 I think this came up previously but I didn't see it when scrolling the thread. @phryneas is right though. The answer to this is just to define the default outside of your function definition, e.g.
const defaultFoo = {};
const defaultBar = () => {};

function example({ foo = defaultFoo, bar = defaultBar }) {
  // ...
}

@bvaughn That's also the way I am doing it, but it's slightly more verbose compared to defaultProps which you just assign once and don't worry about it in the component. I am not defending it though, because of TypeScript tradeoffs with that. Tradeoffs everywhere :)

Not sure that it's really that much more verbose is it? 😁
function Example({ foo, bar }) {
  // ...
}
Example.defaultProps = {
  foo: {},
  bar: () => {}
};

vs
const defaultFoo = {};
const defaultBar = () => {};

function Example({ foo = defaultFoo, bar = defaultBar }) {
  // ...
}

I understand that it is possible to define defaults outside of function scope, but thats me and you, but countless thousands of people will spend hours while debugging performance issues after refactoring to default values, without understanding of this downsides. I wanted to point that out. Definitely should be a part of eslint-plugin-react-perf, but not everybody uses eslint anyway, even if they should.

but countless thousands of people will spend hours while debugging performance issues after refactoring to default values

I would argue that something like that is going to be a performance bottleneck 😆 Developers who actually care about performance are well aware of these optimizations patterns in my opinion.

well, not everybody is aware, and we can prevent this issue by design. I'm not saying that it is a bottleneck, but it could lead to hours of frustration.

I think it's mostly just about educating about these basics like what's referential integrity, it's not related to default props, but any props you are passing around toward the "optimized component".
Great article from Kent C. Dodds: https://kentcdodds.com/blog/usememo-and-usecallback/

At least one of the maintainers of eslint-plugin-react seem opposed to the changes to defaultProps and are resisting adding support for default arguments to the rule. Is there anything the React time can do to educate the community about why defaultProps is being deprecated so that we can be sure the tooling is ready for it? https://github.com/yannickcr/eslint-plugin-react/issues/1009

@nathggns I think you've summarized it pretty well in your https://github.com/yannickcr/eslint-plugin-react/issues/1009#issuecomment-510030229.
If the React team is already decided on it, it would be nice to spread some educated consensus (blog post?) so people can start slowly accepting it and shifting toward default props values.

Not sure if this was discussed before, but about this:

In React 0.12 time frame we did a bunch of small changes to how key, ref and defaultProps works. Particularly, they get resolved early on in the React.createElement(...) call.

Would it make sense to validate prop types at render time?
Consider this snippet:
function Foo({children}) {
  return children;
}
Foo.propTypes = {
  children: PropTypes.node.isRequired
}

ReactDOM.render(cloneElement(<Foo />, {children: 'foo'}), node);

This currently warns, since prop types get validated during createElement and not during rendering.

On the defaultProps removal, are you saying to use native ES2015 for props definitions?
const TestComponent = ({
  array = [],
  bigLongString = '',
  isOpen = false,
}) => (
  // ...

@Sawtaytoes See https://github.com/facebook/react/pull/16210 which already implemented warning about not using defaultProps for functional components. And yes, you are supposed to use ES2015 default argument values. Note that you might want to keep non-primitive values in the outer closure, especially when React.memo is used.

RE: https://github.com/reactjs/rfcs/pull/107#issuecomment-466304382

However if you want to attach a ref on a function component you do need to explicitly do something with the ref.
let [myCustomProp, ref, ...domStuff] = props;
doSomething(myCustomProp);
useImperativeHandle(ref, ...);
return
;

Sometimes there's a need to get a ref of a DOM element that sits inside a function component, and this voids the need to do something like useImperativeHandle inside a function component because the imperative handle will be the DOM element.

Would passing a ref through the function component into the DOM node not work?
Is that discouraged or not supported?
Should a differently named prop be used for that, something like innerRef?

function Foo() {
  const {myCustomProp, ...domStuffThatIncludesRef} = props;
  doSomething(myCustomProp);
  return
;
}
function Bar() {
  const fooRefCallback = useCallback((fooElement) => {
    console.log('Got Foo ref, should be HTMLDivElement: ', fooElement);
  }, []);
  return <Foo ref={fooRefCallback} />;
}

That pattern is encouraged as the more common case (useImperativeHandle is rare). So spread is fine. No need for a different name.
The issue happens when you have two different DOM nodes that you spread the same props to. However that is very rare for other reasons so less likely to cause wide spread problems in practice.

The issue happens when you have two different DOM nodes that you spread the same props to. However that is very rare for other reasons so less likely to cause wide spread problems in practice.

This (the same ref object or ref callback passed into two or more host node props) maybe React could catch and warn, at least in DEV mode, via something like host-instance-to-ref Map/WeakMap.

For what it's worth, I consider this.props.children to be a mistake, and I'd rather see the children prop deprecated in favor of the third argument to createElement/the child position in jsx. Children are fundamentally and conceptually different than props, and further increasing the conflation between the two will imo harm the usefulness of jsx.

For what it's worth, I consider this.props.children to be a mistake, and I'd rather see the children prop deprecated in favor of the third argument to createElement/the child position in jsx. Children are fundamentally and conceptually different than props, and further increasing the conflation between the two will imo harm the usefulness of jsx.

Any prop, including but not limited to the "children" prop, can contain a React Element that in turn can be put into some other elements' props, including but not limited to "children" prop.
Props are inputs to a component that produces elements as output, "children" is one of such inputs.
@ljharb Where do you see the fundamental difference? How would you propose to consume "children" from within a component if it's a special input, not one of props?

@sompylasar jsx is where i see the fundamental difference. Without children being special, there's no point in the custom syntax in the first place.
As for how I propose to consume it, we have this.props, this.state, and this.context, why not this.children?

As for how I propose to consume it, we have this.props, this.state, and this.context, why not this.children?

Maybe because we move away from "this.": function Foo(props): React.Node is a React Component.

@sompylasar sure. but <X {...props}><Y /></X> also becomes React.createElement(X, props, <Y />), so it's all up for potential change.

Other than the namespace to access them what do you expect to be different?
The most compelling argument for separating children to me (which jordwalke has also mentioned) is more of an empirical heuristic. React relies a lot on referential identity and bailing out using paths. That approach is most efficient with an S-expression form. E.g. everything is a tuple. However, in practice things tend to change together and there's overhead in allocations so there is value in flattening the data structures into something like props.
In practice, it happens that it's quite common for values inside children to change while other props doesn't. In this case it would be useful to reuse the old props instance. A tuple of props and children means you can reuse the props while changing the children, which then means you can bailout on the props as a whole unit instead of individually.
It's not always that they change independently. Quite often they change together or maybe there are no props other than children. It can also happen that other props contain JSX elements that are effectively children and those cases could benefit from the same thing. So there's nothing strictly special about children. This argument hedges on the idea that this special case would occur often  enough to justify everyone paying for an extra field/argument.

@sebmarkbage what are your current thoughts on ref extraction?  But in the experimental code, it seems to have special logic

@sebmarkbage my mental model here doesn’t consider performance at all, but what the things are. The sole value of jsx notation is the mechanism for declaring children, and children are not the same thing as props/attributes - they’re conceptually special. So it feels incumbent on react to make the mental model performant, rather than trying to force mental models for performance.

there's nothing strictly special about children. This argument hedges on the idea that this special case would occur often enough to justify everyone paying for an extra field/argument.

I'd like to chime in and echo @ljharb's sentiments here that the raison d'être of JSX is to have this as a conceptual "special-case", or rather to differentiate between the intent of props -vs- children.
While your points are all sound from the point of view of the React internals, and how it has been structured from a performance standpoint, that's not something directly in line with users of JSX as a public API. Without the props/child distinction, the argument for using JSX over directly nesting function calls degrades. XML is fundamentally a representation of triples, not tuples, and JSX is fundamentally an XML-inspired syntax; this is what differentiates it from the Javascript syntax.

Any prop, including but not limited to the "children" prop, can contain a React Element that in turn can be put into some other elements' props, including but not limited to "children" prop.

This is technically true, but pointless and—arguably—conceptually broken. Compare the following:
const X = ({ kids }) => (
{kids}
);
const Y = ({ children }) => (
{children}
);

The X example is possible and usable, but what's the point. Applying it is limiting:
...
const header =
Hi
;
return <X kids={[header]} />
...
// vs
...
return <Y>
Hi
</Y>;

I don't know exactly why JSX was created, but the latter example is in my mind the reason it's used. Just because you can pass components as non-children-keyed props and render them as children of subcomponents, doesn't make that necessarily a good idea for users of React or JSX. Yes it can have it's edge use cases, but they tend to be at a level of component abstraction where the concepts behind JSX-usage are too many layers removed from rendering for it to matter.
This change to children would also cause the elegantly loose coupling between React and JSX to become much tighter. It would not only break any alternative use of JSX, it would remove a lot of the current alternative use-cases, effectively changing it from a syntax designed for React to a syntax designed for React-only.
On the other hand, I would give a 👍 to the suggestion to remove the current special-case of children-as-props-key, in favour of children being isolated as something dedicated. i.e. to deprecate the following usage:
...
const header =
Hi
;
return <Y children={[header]} />;

Regarding the deprecation of defaultProps, I support this in theory, but in practice it is actually very useful. Consider this code:
import styles from '../';
import config from '../';

const MyModule = ({ title, ...props }) => {
  ...
}

MyModule.defaultProps = { styles, config }

...without defaultProps, I would have to rename my styles and config imports, unless I'm mistaken, and end up with code that is more verbose, which seems counter-intuitive.

I have a question i haven't yet seen answered (please excuse if i've just overread it, i wasn't able to read everything), it is perhaps related to the unresolved issue.

In a later major, we'd stop extracting key from props and therefore props is now just passthrough.

If props become passthrough, with key being a new keyword in some way:
Wouldn't that mean that key is now a valid passed prop in React when spread in via {...props}?
Because something like

could effectively lead to

.
(proposed@key syntax for highlight)
This behaviour (@key keyword vs key prop) might seem logical if keys didn't already exist.
But I think it's clear that this isn't wanted because it is confusing and might break lots of old code with different assumptions, that wasn't changed in the upgrade stage.
However, to restrict the use of key as a normal prop, we would still have to dynamically check for a key prop and remove it and issue a warning, like what is described for the upgrade path stage.
However, then we would not fix the described issue:

The problem with this is that we can't statically know if this object is going to pass a key or not. So for every set of props, we have to do an expensive dynamic property check to see if there is a key prop in there.

I might have overlooked something, but currently i don't see how we could get rid of this dynamic check in the described way.

Regarding removing defaultProps, what is the preferred way to fix this simple component?
https://codepen.io/whitelizard/pen/gOpWKOv
I don't know if that is the best example, but generally when destructuring and using complex defaults ([]/{}) with dependency hooks.
defaultProps solves this when destructuring the props, but of course not if something else is destructured in the component body.

@whitelizard
const defaultItems = [];
function App({ items = defaultItems }) {

This pattern is shady, like any code that tries to “sync” state to props. We generally don’t recommend it. If you don’t want items to be individual per render, you can hoist them to a top level variable. Then it will be shared by all renders of every component instance. Or maybe you want to put it in state — then each object will be isolated.

Ok. Well, we have multiple examples of this in different forms that update records.
A current record is fetched and entering as prop, and then we want to keep local form state of the different parts of that record. Say that an asset comes in containing channels, we need to use that destructured prop to set our local form state, and the user can update the list of channels, save etc.
How can I avoid "sync state to props"?

Actually, a useMemo would perhaps be a better example, since the actual problem is what I call "complex defaults". There is no problem with numbers, booleans or strings in hook dependencies, but as soon as there is an object or array, the default empty object or array will trigger any hook with dependency on that object/array (useEffect, useCallback, useMemo).

Sorry, this is the wrong place for this kind of discussion. Thank you for answering related to defaultProps.

@gaearon it feels like the question @whitelizard asked about defaultProps could use a more detailed discussion & is relevant to the RFC.
The pitfalls of the example in the original comment from @whitelizard are definitely non-obvious to beginners & perhaps even the more experienced programmers not actively thinking about this type of stuff.  It is often tempting to default the value at the top to avoid truthy checks later on.
Are you recommending that people don't default values of non-primitive types at all?  Or are you saying that declaring a constant (e.g. const empty = []) at module scope is the recommended approach?  If it's the latter, have you considered having some of the more common constants (e.g. empty array/object, noop function) defined as some sort of a React util?
Would a lint rule to discourage array/object literals/function declarations as defaults in arguments of a function component be a good idea for the community going forward?

I guess this linking rule could be related to eslint-plugin-react-perf. Can you file an issue there? It already includes rules forbidding passing new objects and arrays as props directly, etc.

I now have this in my own react utils:
export const emptyObj = Object.freeze(Object.create(null));
export const emptyArr = Object.freeze([]);

I use them as defaults in all components taking non-primitives. I really like the idea of an eslint rule.

@JustFly1984 thanks for bringing up the eslint-plugin-react-perf! I was not aware of it.  Looking at it more closely, I do appreciate the need for the recommended rules in some contexts, but I know I would hesitate to adopt it for the entirety of our app, just given the risk of premature optimizations.
I am hoping that the rule I am thinking of would have more universal applicability, and would hope that it could land in the main react plugin, assuming it's the correct way to go...

Regarding the defaultProps being deprecation in FCs: I'm just trying to come up with a way to keep all the props in one variable (e.g. props) while also setting the defaults. I used to manage that by setting the defaults while destructing props variable in the FC's body.
I'm currently trying to generate documentation to my component using storybook's Docs Addon with preset-typescript and it doesn't pick up the default when I set it while the FC's is running (as described above). So I landed on using defaultProps.
I'm also pretty sure the said approach caused me a different problem as well, but I cannot remember what it was now. I cannot grasp the whole inner workings of it right now but I suppose doing the following:
const Component = (props) => {
  const { x = 0 } = props;
  ...
}

is not exactly the same as doing
const Component = ({ x = 0 }) => {
  ...
}

or
const Component = (props) => {
  const { x } = props
  ...
};

Component.defaultProps = { x: 0 };

in regard to the component's life cycle. Or is it?
This is why I would object to the deprecation of defaultProps.

@kizivat The first two examples you listed look functionally equivalent to me.
The third one does not, since defaultProps is a React-specific concept.

I would also object (although open to education if I have blind spots here) to the deprecation of defaultProps -- specifically the idea that just using JS default arguments is equivalent. It is considerably less elegant/ergonomic.
In particular, deprecation of defaultProps on function components yields  unnecessarily verbose definitions in TypeScript. Right now I can:
export interface MyProps {
  a: boolean;
  b: string;
  c: number;
  ...etc lots of them
  z: number[];
}

export const MyComponent: React.FC<MyProps> = props => {
  return <somethingInteresting {...props} /> // obviously a vacuous example
}
MyComponent.defaultProps = { a: true, b: "yes", c: 1 }

With the new syntax wouldn't I have to name every prop in the MyComponent definition (or destructure as "defaulted props" and "the rest") to use any default prop in an FC?

In particular, deprecation of defaultProps on function components yields unnecessarily verbose definitions in TypeScript. Right now I can:
...
With the new syntax wouldn't I have to name every prop in the MyComponent definition (or destructure as "defaulted props" and "the rest") to use any default prop in an FC?

Sorry, maybe I'm to sleepy to understand what the perceived issue is. But your example could be written without defaultProps as one of the following. Neither of which require naming every prop in the function definition or using a ...rest.
export interface MyProps {
  a: boolean;
  b: string;
  c: number;
  ...etc lots of them
  z: number[];
}

export const MyComponent: React.FC<MyProps> = props => {
  return <SomeOtherComponent a={true} b='yes' c={1} {...props} /> // obviously a vacuous example
}

export interface MyProps {
  a: boolean;
  b: string;
  c: number;
  ...etc lots of them
  z: number[];
}

export const MyComponent: React.FC<MyProps> = props => {
  const otherProps = {
    a: true,
    b: "yes",
    c: 1,
    ...props
  }
  return <SomeOtherComponent {...otherProps} /> // obviously a vacuous example
}

@bvaughn I suppose, you're right. The difference between the first two though is that although the 1st exposes props for further use, without the default value for x - that is a better articulation of the problem with the defaultProps deprecation I see).
That may result in some unexpected behavior. See https://codesandbox.io/s/mystifying-burnell-sfrns?file=/src/App.js

@dantman the version you posted works fine for the behavior of default props. That's not my issue, there are of course a myriad of ways to solve that problem. I'm talking about ergonomics, opportunity for human error, inspectability by documentation generation tools, etc. defaultProps serves a useful purpose and I don't think "well you can get the same behavior with destructuring and js argument defaults" covers the entire proposed loss.

What @dantman proposed solves my use case in my comment
#627700529 However, as @craigkovatch states above, I find it human error-prone and also, as per my earlier use case - e.g. @storybook/preset-typescript cannot pick up the defaults from the body of a FC.

I'd like to call out, in re the defaultProps depreciation, that defaultProps is a necessary element in documentation workflows that rely on react-docgen, which is unable (as far as I know) to pick up defaults from argument fallbacks. Eliminating defaultProps would break my ability to accurately document the libraries I maintain at work.

react-docgen, which is unable (as far as I know) to pick up defaults from argument fallbacks

@chadlavi  latest versions should work. If you have special patterns you can write your own resolvers. We're using react-docgen in Material-UI and don't use .defaultProps anymore. Maybe open on issue in react-docgen if that's not working for you?

react-docgen, which is unable (as far as I know) to pick up defaults from argument fallbacks

@chadlavi latest versions should work. If you have special patterns you can write your own resolvers. We're using react-docgen in Material-UI and don't use .defaultProps anymore. Maybe open on issue in react-docgen if that's not working for you?

@eps1lon this is great to hear! (We use MUI by the way, thanks!) I'll check with my team. We've actually started doing the realllllly annoying move of sometimes doing BOTH a default value AND a defaultProps key, one to satisfy TS, one to satisfy react-docgen. Extremely unDRY and annoying.

I have updated my initial example to hopefully be even clearer regarding the issue as I see it.
The question, as I see it, is: What the recommended pattern should be, and how to make that clear.
https://codepen.io/whitelizard/pen/gOpWKOv?editors=0010
function App({ items = [] }) {
  const [count, setCount] = React.useState(0);

  const expectedOnce = React.useMemo(
    () => (items.length + 1) * Math.random(),
    [items]
  );

  return (

Count: {count}

Expected to not update: {expectedOnce}

      <button onClick={() => setCount(x => x + 1)}>Inc</button>

  )
}

Isn't the obvious fix to this just to hoist items into module scope?
const defaultItems = [];
function App({ items = defaultItems }) {
  const [count, setCount] = React.useState(0);

  const expectedOnce = React.useMemo(
    () => (items.length + 1) * Math.random(),
    [items]
  );

  return (

Count: {count}

Expected to not update: {expectedOnce}

      <button onClick={() => setCount(x => x + 1)}>Inc</button>

  )
}

Yes, that is what I already wrote earlier in this thread.
But how about something like:
function App({ items = React.default([]) }) { // or React.emptyArray, or similar
  const [count, setCount] = React.useState(0);
  ...
}

Just thinking if React could help out in this, due to that the issue seems bound to React hooks dependency checking mainly?

I suppose a benefit of something like React.default would be it could be statically analysed and the linter could (possibly) ensure non-reference types are not including in default values. I like it.

In my opinion, it adds unnecessary overhead and clutter when you can achieve the same by declaring default out of scope. Sure, it's not JS newcomer friendly, but once they understand such basic behavior, they will be fine with it.
Personally, I even have a single EMPTY_ARRAY or EMPTY_OBJECT to make it more apparent in the code and easily traceable.

react-docgen, which is unable (as far as I know) to pick up defaults from argument fallbacks

@chadlavi latest versions should work. If you have special patterns you can write your own resolvers. We're using react-docgen in Material-UI and don't use .defaultProps anymore. Maybe open on issue in react-docgen if that's not working for you?

Despite that being true (react-docgen does indeed support retrieving default values for props as set in the arguments), I do want to point out that - barring the use of typescript - it will be impossible to infer other characteristics of the props when solely using ES6 destructuring syntax and defining the prop default values in the arguments, such as the type (boolean, string, function?) or if they are required or optional, while this is essential for documenting components.
At least, at this point, react-docgen (and ast-types) do(es) not cater for that need.

I just wrote this entire article about this subject.  TLDR: I hate the idea of removing defaultProps for functional components because, IMHO, it causes major headaches.  And no, simply using object destructuring or inline defaults is not an equivalent solution.
https://dev.to/bytebodger/default-props-in-react-typescript-2o5o

@semanticbodger I only skimmed your article so apologies if you can't do it this way, but the solution I'd go for is this
before:
export default function MyJSComponent(props) {
   return (
      <>
         Here is MyJSComponent:

         {props.children}
      </>
   );
}

MyComponent.propTypes = {
   requiredString: PropTypes.string.isRequired,
   requiredNumber: PropTypes.number.isRequired,
   optionalBoolean: PropTypes.bool,
   optionalString: PropTypes.string,
   optionalNumber: PropTypes.number,
};

MyComponent.defaultProps = {
   optionalBoolean: true,
   optionalString: 'yo',
   optionalNumber: 42,
};

after
import React, { FC } from "react";

export default const MyTSComponent: FC<{
   requiredString: string,
   requiredNumber: number,
   optionalBoolean?: bool,
   optionalString?: string,
   optionalNumber?: number,
}> = ({
   requiredString,
   requiredNumber,
   optionalBoolean = true,
   optionalString = 'yo',
   optionalNumber = 42,
   children
}) => {
   return (
      <>
         Here is MyJSComponent:

         {props.children}
      </>
   );
}

MyComponent.propTypes = {
   requiredString: PropTypes.string.isRequired,
   requiredNumber: PropTypes.number.isRequired,
   optionalBoolean: PropTypes.bool,
   optionalString: PropTypes.string,
   optionalNumber: PropTypes.number,
};

MyComponent.defaultProps = {
   optionalBoolean: true,
   optionalString: 'yo',
   optionalNumber: 42,
};

Thank you, @nathggns.  And yes, I basically covered that as one of the options in my article.  What you've shown works.  But IMHO it falls quite short of what we have with defaultProps.  To simplify this, I'm going to show a shorter version to highlight my points:
import React, {FC} from "react";

export const PropTest: FC<{
   required: string,
   optional?: string,
}> = ({
   optional = 'default',
   ...props
}) => {
   return (
      <>
         required = {props.required}

         optional = {optional}

         {props.children}
      </>
   );
};

In this example, I'm using ...props to allow me to reference props.children as well as the required variable.  The problem with this approach is that required, optional, and children _are all props.  And yet, I'm referring to optional_without_ theprops.scope, whilerequiredandchildren` are still in that scope.  In other words, even though these are all props, we have to reference them in two different ways.

You could "fix" this by specifically destructuring required and children.  This solves the problem of mismatched props. references - by creating another problem.  Destructuring all the props completely removes any reference to props. in the body of the function - and that's not a good thing.  It's extremely useful to be able to look through the body of any functional component and know exactly which variables are props - because they're prefixed with props..  Also, taking this approach is very unwieldy if we have a longer set of props on the component, because we have to define all the types in FC<{...}>, and then we have to account for every one of those variables again in the prop destructuring.

The simple fact is that I haven't yet seen a "solution" for this that doesn't lose at least some of the important functionality that you get with defaultProps.  There seems to be this misguided idea that:

Functions already allow for the setting of default values, so we don't need defaultProps on functional components.

But this is flawed.  Specifically, it falls down when you're passing in an object (and props are always passed in as a single object), and you want to maintain the reference to that object (e.g. props.) inside the function body.

I’ve worked on more React code based than I can count in my career and not one of them leaves the props in an object in that way for function components. I guess you can do that, but it’s clearly fighting against the current, so I guess don’t be surprised if you run into issues?
On Sat, Jun 20, 2020 at 12:05 PM GitHub<EMAIL_ADDRESS>wrote:
“Thank you, @nathggns. And yes, I basically covered that as one of the options in my article. What you've shown works. But IMHO it falls quite short of what we have with defaultProps. To simplify this, I'm going to show a shorter version to highlight my points:
import React, {FC} from "react";
export const PropTest: FC<{
required: string,
optional?: string,
}> = ({
optional = 'default',
...props
}) => {
return (
<>
required = {props.required}
optional = {optional}
{props.children}
</>
);
};1. In this example, I'm using ...props to allow me to reference props.children as well as the required variable. The problem with this approach is that required, optional, and children are all props. And yet, I'm referring to optional_without theprops.scope, whilerequiredandchildren` are still in that scope. In other words, even though these are all props, we have to reference them in two different ways.
2. You could "fix" this by specifically destructuring required and children. This solves the problem of mismatched props. references - by creating another problem. Destructuring all the props completely removes any reference to props. in the body of the function - and that's not a good thing. It's extremely useful to be able to look through the body of any functional component and know exactly which variables are props - because they're prefixed with props.. Also, taking this approach is very unwieldy if we have a longer set of props on the component, because we have to define all the types in FC<{...}>, and then we have to account for every one of those variables again in the prop destructuring.
The simple fact is that I haven't yet seen a "solution" for this that doesn't lose at least some of the important functionality that you get with defaultProps. There seems to be this misguided idea that:
“Functions already allow for the setting of default values, so we don't need defaultProps on functional components.”
But this is flawed. Specifically, it falls down when you're passing in an object (and props are always passed in as a single object), and you want to maintain the reference to that object (e.g. props.) inside the function body.”
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.

I feel like you’re fighting against the current by leaving props in an object like that. I don’t do it and I’ve not yet worked on a React code base that does.
My issue with defaultProps is that is puts the defaults for the prop far off from where you’re receiving the props, which can be really confusing.
These components are also difficult to type, because inside the component the prop will never be undefined, but you have to specify the type as including undefined so that you don’t need to pass it when you use the component.

I feel like you’re fighting against the current by leaving props in an object like that. I don’t do it and I’ve not yet worked on a React code base that does.

I imagine that this might be a difference of those who do more JS-based React, versus those who do more TS-based React.
You may not do this.  You may not have worked on a code base that does.  That doesn't make it "fighting against the current".  It's outlined right in the core docs as a pattern.  You can see it here:
https://reactjs.org/docs/components-and-props.html
Specifically, it shows this as an example:
function Welcome(props) {
  return
Hello, {props.name}
;
}

In fact, there are 8 separate examples on that page where they show this exact pattern.  If they outline it so prominently in the core docs, I can pretty much guarantee that I'm not the only dev in the world who's chosen this strange and foreign approach.

FWIW: I converted my Typescript React code which looked like this:
export interface MyCompProps {
   p1?: () => void;
   p2?: string[];
   p3: number;
   p4: string;
}

export const MyComp = React.FC<MyCompProps> = ({ p1, p2, p3, p4 }) => { ... };

MyComp.defaultProps = {
  p1: () => null,
  p2: []
};

to now look like this:
export interface MyCompProps {
   p1: () => void;
   p2: string[];
   p3: number;
   p4: string;
}

const DEFAULT_PROPS: Partial<MyCompProps> = {
  p1: () => null,
  p2: []
};

export const MyComp = React.FC<MyCompProps> = ({ p1 = DEFAULT_PROPS.p1!, p2 = DEFAULT_PROPS.p2!, p3, p4 }) => { ... };

Some things to note

I add a pling (exclamation mark) to the end of the default props on assignment (e.g. p2 = DEFAULT_PROPS.p2!). This is deliberate as it tells the Typescript compiler that the optional parameter (p2 in this case) definitely has a value and can therefore be safely dereferenced (if necessary) in the code block that follows.

You may think that you could do away with DEFAULT_PROPS and simply assign the defaults directly, like this:

export const MyComp = React.FC<MyCompProps> = ({ p1 = () => null, p2 = [], p3, p4 }) => { ... };

However, I found that this causes a very large number of re-rendering events as I /think/ that React thinks you have now changed the value of the optional parameter to something else. Using DEFAULT_PROPS was the only way I could stop this from occurring as it creates a fixed reference to these default values.

However, I found that this causes a very large number of re-rendering events as I /think/ that React thinks you have now changed the value of the optional parameter to something else. Using DEFAULT_PROPS was the only way I could stop this from occurring as it creates a fixed reference to these default values.

This is because of your e.g. p1 = () => null, which creates a new anonymous "return null" function on every reader. You would need to move this into a not-inlined named function or const to avoid this behavior, which is pretty much equivalent to using DEFAULT_PROPS

This is because of your e.g. p1 = () => null, which creates a new anonymous "return null" function on every render. Or more fundamentally -- those default parameters are not created when you declare them in the function signature. Those default parameter values are a TypeScript construct, so when they get transpiled into JS, they're being created inline "anew" at the top of each MyComp function call.

You would need to move these into a not-inlined defaults to avoid this behavior, which is spritually equivalent to using DEFAULT_PROPS.

Thanks for the clarification - much appreciated.

I hope I'm not repeating that no defaultProps in function components also means that you need to be very disciplined with usage of direct property access on props. If you access props.disabled then you won't get the default value. This might seem trivial (and can be linted against with react/destructuring-assignment) but it's still something to keep in mind.

I really object to removing defaultProps, arguments deconstruction causes much frustration for me when components have many props have many defaultValues and  re-composition props as ever having seen above examples and miss the reconstructing needed props with human error without typing (of course though everyone has typing appropriately it is not problem).
In my perspective, if removing defaultProps, we can't use declarative props in as object and it lose (at least we must  spend reconstruction props cost for this) some strong typing feature to complete code intellisense from props. I understand less code React Component is preferable and it is not very important feature if you maintain short code
react component but I faced many cases someone writes enormous components and I have trouble to know what props  is defined from component code lines, then I check many times props list from arguments.
Further more, If decomposition props and, it reassign another variable in the code, we often get confused and which
we should use these parameters derived from props, or internal scoped variables.
I'm using defaultProps for ui library style setting like

type DefaultListProps =  Pick<ListProps, 'itmsPerPage' | 'listProps'>
const defaultProps: DefaultListProps = {
  itemsPerPage: 10,
  listProps: {
    height: 600,
    width: '100%',
  },
}

List.defaultProps = defaultProps

This is worth that storybook components avoid to re-defined same props and,  mess for intrinsic no-need definition in function and code writing this way  is portable because easily copy and paste though little those who use so.
and these makes it easily props injection for like theme props as it used in styled-compnents
In additional, this make it partial automatically in typescript even if List component required types.
Props merging process in lifecycle seem not spend so much cost compared to remove it.
If it is decided to remove in react for performance optimization, It is understandable, but I will make it same utility inserting merging props with hoc. So, I want to remain the hatch if you can.

I'm not quite understanding @tkow, so I might be duplicating what we already know, but my concern with removing defaultProps is one of documentation. There's not a good 1:1 translation between the two.
Take a look at these three examples:

The second and third examples are functionally the same, but the first is different. If React renders <MyComponent /> again, it will create a new default object for value.
Am I understanding this correctly?
What happens if you have useEffect calls like this:
const [
  localValue,
  setLocalValue,
] = useState(value)

useEffect(
  () => {
    setLocalValue(
      value
    )
  },
  [value],
)

return (

    {value.text}

)

The first example would call the useEffect more times than you expect whereas the latter two only call it the one time (at mount).  While you may think "a few extra Virtual DOM diffs is no big deal", it will lead to many unexpected bugs in hooks because you're writing with the intention that props, which were never passed, are never changing.
I myself have run into this issue as well and kept wondering why my values kept getting overridden when children updated even though I wasn't passing any props.

In 99% of cases, default props are usually things like null, '', false, etc. Those will translate over fine. It's objects that will have issues including arrays, maps, and sets.
Complex objects aside, you could easily set some defaults as Object.prototype, Array.prototype, and Function.prototype (I already use these in production), but you have to be very careful not to do any mutations. Works fine for my team and should work for all teams, but keep that in mind as many developers think it's safe to mutate in a .reduce.
Complex objects require complex initialization by setting a static variable outside of the function to ensure the object reference doesn't change.
Just about any other JS data-type is going to be problematic though; requiring initialization variables. An easy example are Map and Set as you can't do new Set(Set.prototype).
Here are some others which do or may not work cleanly with Class.prototype:

BigInt
Date
RegExp
Any other native object type.

It's probably too late for a substantial change to this proposal, but:
I'd like to see the transform implemented this way:
    function foo(props) {
      return
hi!</ht>
    }

Becomes
    function foo(jsx) {
      return function foo(props) {
        return  jsx('h1', ...);
      }
    }

Then, the first time a given component is rendered your view framework injects the right value of "jsx" and caches the resulting Component. This means no static dependencies are necessary to write components for libraries that use JSX.

How would that work with declaring JSX outside of a component?
// in
const jsx =

// out
const jsx = (jsx) => jsx('div')

like this? This would lead down a slippery slope of a really complex transform, with lots of little edge cases to learn about on the user end, wouldn't it?

That could be a problem, although it could be mitigated by picking an identifier that's unlikely to collide (__jsx_html___ or something) and documenting this prominently.  I think, personally, I'd prefer this to having an implicit import statement added to my code, which has similar name-collision problems anyways.

copying and pasting my HN comment here:
This is a missed opportunity to make JSX more than solely about UI and I'm pretty disappointed about it.
JSX is really a way to describe a hierarchy of deferred function calls independent of their callers. This could be tremendously useful in a number of different contexts, not just UI. What would a state machine look like if the states could be described in JSX? What would rx.js look like if this mechanism was available?
The current tooling makes experimenting with possible alternate use cases pretty awkward, and so basically no one is doing work in this area.
I believe that these changes, particularly the jsx and jsxs auto import, will make it even harder to separate JSX as a concept from React and React-like libraries. It'll make it harder to standardise JSX, which is frustrating as it's the most supported ECMAScript syntax extension by a long way.
My modest proposal is that a JSX element desugars to a call to a block scoped identifier called jsx, so I can do something like this:
    import { jsx } from 'react';
    import { elementCreator } from 'somelib';

    const someFunction = () => {
      const { jsx } = elementCreator();

      return <iAmNotAReactElement />;
    };

    const MyReactComponent = () =>
cool
;

Taking this alternative approach would give developers a widely supported syntax extension that could allow for some really innovative solutions. The current path is going to do the opposite.
I understand that there are perf optimisations that React can do if they control all of this. But I feel like they're looking at this too narrowly, from only the perspective of UI. They have the power to open up a lot more opportunities.

To further support what @phpnode is saying, I would add that even if you wanted to make jsx a UI-only concept, you could easily imagine having multiple JSX-aware libraries coexist in a codebase.  React 16 allows that to happen, as long as the these libraries don't have to coexist in the same module. React 17 would make it a lot more difficult, because the webpack config would basically need to designate which JSX plugin to use for which files...

@Sawtaytoes the same concerns were brought up at https://github.com/reactjs/rfcs/pull/107#issuecomment-624353424 & in the referenced comments above it.  I have not seen anyone from the react team weigh in on the proposed solutions for defaulting complex types to avoid the pitfalls you bring up.   My personal opinion is that an eslint rule could go a long way toward discouraging people from doing what you have in Example 1.  I've personally seen a number of developers get tripped up by this problem, and it's led to a number of hard to find bugs.

I know this is another late comment, but want to voice my concern here regarding the deprecation of defaultProps.

Developers currently have the freedom to choose whether or not to destructure in the function constructor.  If I'm reading this RFC correctly, this will remove that choice and require everyone to destructure and set defaults in function constructors.

I want to echo @semanticbodger's concern in his article about the issues concerning TypeScript.  It would mean that this pattern would go away, forcing default values to clutter up the function signature (also forcing us to be clear about defining children, which I don't have an issue with).

This isn't a big deal with components that accept only a few props.  This is what we'd write with TypeScript and destructuring, which seems pretty clean:
export type ButtonProps = {
    /** Whether this is a primary or secondary button.  */
    primary?: boolean;
    /** Button size. */
    size?: 'small' | 'medium' | 'large';
}

export const Button = ({ primary: false, size: 'medium' }: ButtonProps): ReactElement => {
    //...
}

But with a more complicated component that accepts many props, the function signature becomes much more confused:
export type ButtonProps = {
    //...
}

export const Button = ({
    children,
    classes,
    className,
    color = 'primary',
    component = 'button',
    disabled = false,
    disableElevation = false,
    disableFocusRipple = false,
    endIcon: endIconProp,
    focusVisibleClassName,
    fullWidth = false,
    size = 'medium',
    startIcon: startIconProp,
    type = 'button',
    variant = 'text',
    ...other
  }: ButtonProps): ReactElement => {
    //...
}

Right now I'd much rather do the destructuring inside the function itself, to keep the function signature itself clean and more readable, like this:
export const Button = (props: ButtonProps): ReactElement => {
    const {
        children,
        classes,
        className,
        color = 'primary',
        component = 'button',
        disabled = false,
        disableElevation = false,
        disableFocusRipple = false,
        endIcon: endIconProp,
        focusVisibleClassName,
        fullWidth = false,
        size = 'medium',
        startIcon: startIconProp,
        type = 'button',
        variant = 'text',
        ...other
    } = props;
    //...
}

I suppose it could be said this is less of a concern in IDEs and more of a concern for code maintainability as well as readability and code reviews, which will suffer a bit in the meantime.

Here is bable plugin for those who need to have access to es6 default parameters outside component 👍

Here is a babel plugin for those who need to have access to the es6 default parameters outside of the component 👍

I found solution for defaultProps decomposition problem when making functional a component stress free as possible. Making function like custom hook to use default Props like
function useDefaultProps
(props:  P): P => ({
  example: props.example || ''
})

and use it like

function SomeComponent (incomingProps: P){
    const props =
useDefaultProps(incomingProps)
}

This way also has advantage explicitly for arbitrary merging nested reference and comfinement. It has capablility to make partial props type to restricted when modifying above return P type props code to Required .
So I found conclusion that we can avoid lacking of feture by decomposition.

this is the best option for me until now, it works nicely... autocomplete is working fine and props defined on defaultProps are marked as required for internal component use (works better with typescript 4.2).
export const useDefaultProps = <
  Props extends object,
  Defaults extends [K in keyof Props]?: Props[K] = {}
>(
  props: Props,
  defaultProps: Defaults
) => {
  const mergedProps = {
    ...defaultProps,
    ...props,
  };

  return mergedProps as Props & Required<Defaults>;
};

i use it like:
interface P {
    someProp: string;
    anotherProp?: number;
}

function SomeComponent (incomingProps: P){
    const props = useDefaultProps(incomingProps, {
        anotherProp: 17
    });
}

this is the best option for me until now, it works nicely... autocomplete is working fine and props defined on defaultProps are marked as required for internal component use (works better with typescript 4.2).
export const useDefaultProps = <
  Props extends object,
  Defaults extends [K in keyof Props]?: Props[K] = {}
>(
  props: Props,
  defaultProps: Defaults
) => {
  const mergedProps = {
    ...defaultProps,
    ...props,
  };

  return mergedProps as Props & Required<Defaults>;
};

i use it like:
interface P {
    someProp: string;
    anotherProp?: number;
}

function SomeComponent (incomingProps: P){
    const props = useDefaultProps(incomingProps, {
        anotherProp: 17
    });
}

useDefaultProps as described above is not really a hook so its name is quite misleading

useDefaultProps as described above is not really a hook so its name is quite misleading

Hi! What would the suggested alternative be to not using defaultProps when a component needs to read the props of it's children?
We have a bunch of "wrapper" components that add functionality to it's children. For example, overloading the child's onClick prop to do an additional callback, but keeping the child onClick prop callback as well (even if it comes from default values). See the examples here.
This was previously mentioned by @eps1lon but no clear migration path was suggested. I do not understand how I should remove defaultProps usage in my library

Hi! I was reminded of this after reading https://www.reddit.com/r/reactjs/comments/n51ly2/what_is_one_thing_you_find_annoying_about_react/gwz6edj. With the changes introduced by the new JSX automatic transform, are we any closer to removing the need for forwardRef? 😄

Here English translation using google

Deprecate defaultProps on function components.

对于这个提案，我从开发的角度有一些看法：

当使用 es6 解构时，会存在以下问题需要解决：

解构会使得 useEffect 监听不必要触发（对于引用类型的 props，比如：[] ）？

解构会使得当前组件无法被其父组件 React.Children.map 获取 props 默认值？

当使用 defaultProps 时，会存在以下问题需要解决：

defaultProps 类型如何保证和 props 一致？比如字符串联合类型，传递字符串需要 as 对应的联合类型

defaultProps 在组件内部使用的时候需要为其抹除 undefined

Another reason I found defaultProps being better than default parameters is preserving the references of objects and arrays.
const Component = (props) => {};
Component.defaultProps = {
  items: [],
};
// vs
const Component = ({ items = [] }) => {};

If Component is responsible for rendering memoized children, or simply using items in a dependency of some hook, a default parameter will create a new reference every render (if items is not provided). If you're using defaultProps, it's always the same reference.
I couldn't find an alternative that looks better than defaultProps did.
const defaultItems = [];
const Component = ({ items = defaultItems }) => {};
// Imagine adding more props using a pattern like this. Yikes

const defaultProps = {
  items: [],
};
const Component = ({ items = defaultProps.items }) => {};
// Now I'm just implementing the same logic React did for me with defaultProps anyway

Having a handy built-in feature for this is the most simple and elegant solution imo.

@sebmarkbage , @eps1lon, @gaearon, is there any plan to implement this in the near future? At Microsoft, we use React heavily and have noticed that createElement is a big performance bottleneck for us. I also looked at performance profiles from Facebook and Discord and saw similar performance bottlenecks. The cost of cloning the config object in createElement when it doesn't need to be cloned 99%+ of the time, results in a large amount of memory churn from the config not getting re-used and increased latency from looping through the config object and reassigning the key value pairs (excluding the key and ref) to the new props object. I created an issue here -> https://github.com/facebook/react/issues/27216.
We've tested a change where the compiler picks a new createElement function that assigns props to config and just deletes the reserved properties. We see a 3-4% reduction in latency in common scenarios such as a Microsoft Teams chat switch, and a 90% reduction in memory created from createElement (including memory discarded by Minor GC and Major GC).
Since the scope of this RFC is very large, is it possible to break this RFC into two phases where in phase 1 we fix the performance issues that don't cause breaking changes and integrate them into the jsx compilers, and in the following phase 2 we include the other deprecations that @sebmarkbage has proposed to make createElement even simpler.
I'm proposing that for phase 1 we create a new createElement function where the compiler will decide if it should be used or not.
It will look something like this:
export function createFastElement(type, config, children) {
  let propName;
  // Reserved names are extracted
  // const props = {};
  let props = config; // re-use the config
  let key = null;
  let ref = null;
  let self = null;
  let source = null;

  if (props != null) {
    if (hasValidRef(props)) {
      ref = props.ref;

      if (__DEV__) {
        warnIfStringRefCannotBeAutoConverted(props);
      }
    }
    if (hasValidKey(props)) {
      key = '' + props.key;
    }

    self = props.__self === undefined ? null : props.__self;
    source = props.__source === undefined ? null : props.__source;

    // // Remaining properties are added to a new props object
    // for (propName in config) {
    //   if (
    //     hasOwnProperty.call(config, propName) &&
    //     !RESERVED_PROPS.hasOwnProperty(propName)
    //   ) {
    //     props[propName] = config[propName]; // this is the most expensive part of the operation
    //   }
    // }

    // Reserved properties are deleted in place
    for (propName in RESERVED_PROPS) {
      if (hasOwnProperty.call(props, propName)) {
        delete props.propName; // only called when there is a ref or a key in the config object.
      }
    }
  } else {
    props = {};
  }
...

For Case 1 the compiler will continue to use the existing createElement

For Case 2 the compiler will use the createFastElement function.

The problem we have is that there's not one single uniformly used compiler for React. It takes a long time for a compiler change to trickle through the community. It was only recently that people started adopting the new one. The point of the new one is that the same compiler is compatible with the end state.
If we released this intermediate step, people would have to switch away from the compiler that supports the end state to a new one that doesn't. We'd at least have to structure it so that it's compatible with the end state. However, even if we did that, we'd end up with an extra intermediate runtime that we'd have to support indefinitely even after shipping the end state because it would take a long time to phase out the intermediate state compiler.
That is in addition to any further runtime support for the larger React Forget compiler project which will also add to go further.

@sebmarkbage for an intermediary performance fix, what if we modified the existing createElement, so there wouldn't be a need for any compiler changes?
Could we use the ...rest spread instead of doing a for-in loop? It's faster and prevents the new props object from becoming a dictionary mode object resulting in faster cloning time. This is an issue in v8: https://bugs.chromium.org/p/v8/issues/detail?id=14245&q=dictionary&can=2
Option 1:

@@ -2,10 +2,11 @@ export function createElement(type, config, children) {
   let propName;

   // Reserved names are extracted
-  const props = {};
+  let { key, ref, __source, __self, ...props } = config || {};
+
+  key = null;
+  ref = null;

-  let key = null;
-  let ref = null;
   let self = null;
   let source = null;

@@ -23,15 +24,6 @@ export function createElement(type, config, children) {

     self = config.__self === undefined ? null : config.__self;
     source = config.__source === undefined ? null : config.__source;
-    // Remaining properties are added to a new props object
-    for (propName in config) {
-      if (
-        hasOwnProperty.call(config, propName) &&
-        !RESERVED_PROPS.hasOwnProperty(propName)
-      ) {
-        props[propName] = config[propName];
-      }
-    }
   }

Could we also simply assign the config object to the props object when the props object does not need to get modified?
This could potentially break changes if the props object passed into createElement is later mutated by the user's code, but it would throw an error in development mode since the props object is frozen. I feel like that's an anti-pattern that should be avoided anyway.
Option 2:
@@ -2,17 +2,22 @@ export function createElement(type, config, children) {
   let propName;

   // Reserved names are extracted
-  const props = {};
+  let props = {};

   let key = null;
   let ref = null;
   let self = null;
   let source = null;

+  let allowPropsToReferenceConfig = true;
+
+  // Children can be more than one argument, and those are transferred onto
+  // the newly allocated props object.
+  const childrenLength = arguments.length - 2;
+
   if (config != null) {
     if (hasValidRef(config)) {
       ref = config.ref;
-
       if (__DEV__) {
         warnIfStringRefCannotBeAutoConverted(config);
       }
@@ -23,20 +28,31 @@ export function createElement(type, config, children) {

     self = config.__self === undefined ? null : config.__self;
     source = config.__source === undefined ? null : config.__source;
-    // Remaining properties are added to a new props object
-    for (propName in config) {
-      if (
-        hasOwnProperty.call(config, propName) &&
-        !RESERVED_PROPS.hasOwnProperty(propName)
-      ) {
-        props[propName] = config[propName];
+
+    for (propName in RESERVED_PROPS) {
+      if (hasOwnProperty.call(config, propName)) {
+        allowPropsToReferenceConfig = false;
       }
     }
+
+    if (type && type.defaultProps) {
+      allowPropsToReferenceConfig = false;
+    }
+
+    if (childrenLength > 0) {
+      allowPropsToReferenceConfig = false;
+    }
+
+    if (allowPropsToReferenceConfig) {
+      props = config;
+    } else {
+      // Remaining properties are added to a new props object
+      // eslint-disable-next-line no-unused-vars
+      const { key: _key, ref: _ref, __self, __source, ...rest } = config;
+      props = rest;
+    }
   }

-  // Children can be more than one argument, and those are transferred onto
-  // the newly allocated props object.
-  const childrenLength = arguments.length - 2;
   if (childrenLength === 1) {
     props.children = children;
   } else if (childrenLength > 1) {

I took a benchmark of each case and saw that they were much faster than the current createElement:
https://codepen.io/zbogard/pen/QWzpwjy

One reason we don't tend to rely on modern features in React itself is because people end up using compilers downstream that lowers it into a worse and unpredictable version than React itself does. However, at the downside of not being able to use a native version if it ends up faster.
However, I will say that just shipping the real breaking change here isn't that hard for someone with context. We just don't have anyone with context working on it right now. So instead of having them focus on an intermediate, they could just ship the real thing that's even better.


flake: Add GitHub Action to update flake.lock

Sorry for the delayed reply on this. Looks good! I haven't used this action before so I'm curious to see how it is going to work in practice. Based on how it goes we can always make adjustments later if needed.
The main detail I anticipate needing to adjust is the frequency of the update PRs. Every three days seems alright during summer of nix while there are many people working on the repo regularly but that could be greatly reduced for the rest of the year.
I'll go ahead and merge this now!


Use generated proto types from cadence-idl repo

What changed?

go get github.com/uber/cadence-idl
Update Makefile to exclude generated types in idls from formatting
Update Makefile to not generate public API proto types
Removed existing generated public API proto types
Replaced github.com/uber/cadence/.gen/proto/api/v1 with github.com/uber/cadence-idl/go/proto/api/v1 everywhere
Regenerate internal proto types (import path updates, binary descriptor changes)

Why?
Public API proto types are generated on cadence-idl repo (https://github.com/uber/cadence-idl/pull/94) so that they could be shared between client/server. Otherwise it results in duplicated type registrations when using both.

How did you test it?

Potential risks

Release notes

Documentation Changes

Pull Request Test Coverage Report for Build 3a859310-eaae-4bc0-9dfa-36dc48454c39

5 of 5 (100.0%)  changed or added relevant lines in 1 file are covered.
134 unchanged lines in 11 files lost coverage.
Overall coverage decreased (-0.06%) to 56.878%

Files with Coverage Reduction
New Missed Lines
%

service/history/execution/mutable_state_task_refresher.go
1
73.82%

common/task/parallelTaskProcessor.go
2
92.75%

common/task/weightedRoundRobinTaskScheduler.go
2
89.12%

service/history/queue/timer_queue_processor.go
2
58.8%

common/task/fifoTaskScheduler.go
3
84.54%

common/persistence/nosql/nosqlplugin/cassandra/workflow.go
6
55.04%

common/persistence/nosql/nosqlExecutionStore.go
9
60.35%

common/persistence/nosql/nosqlplugin/cassandra/tasks.go
14
72.8%

common/persistence/nosql/nosqlTaskStore.go
16
58.37%

common/persistence/nosql/nosqlplugin/cassandra/workflowUtils.go
21
75.87%

Totals

Change from base Build 41f430f9-2fd6-4e27-8297-0d5ae9092eaa:
-0.06%

Covered Lines:
82348

Relevant Lines:
144781

💛  - Coveralls


Update JSON parsing for cedar#771

Issue #, if available:
Description of changes:
Update Lean parsing of json names to match cedar#771.
Also updated the json files in the ceda-lean/Cli/ folder. They are auto-generated, so no need to review. These tests are useful for local debugging, but I am open to deleting them if reviewers feel like they're cluttering the repo.

Note: DRT failures are expected due to the recently merged cedar#714 and cedar#771.


west.yml: hal_espressif: use common shared source

This fixes an issue related to SPIRAM and DRAM_1 region access. It is possible that the application crashes due to invalid pointer value.

Wait, it needs to be updated.


Error responses using json

Is there a way to make all error responses to be json?
Or is any other way to catch all this in order to force the response to json?
I have seen #211  but can't tell how to use it

All errors are thrown as JSON? https://github.com/lucadegasperi/oauth2-server-laravel/blob/e12b5585fd78e9bb6c1df672891669d017411137/src/Middleware/OAuthExceptionHandlerMiddleware.php#L37-L43

If u are using laravel debug bar, it will not throw the proper exception

https://github.com/barryvdh/laravel-debugbar/pull/409

@vinkla ... I think that is why want.. I didn't know about that middleware.. Should replace oauth for this? or added after it?
@lukepolo I am not using debugbar ... But thanks

Please see the documentation https://github.com/lucadegasperi/oauth2-server-laravel/blob/master/docs/getting-started/laravel-5.md

@vinkla I have same exact configurations...
But when I get an access token that does not exist or is not valid, expirated, Laravel gives me an exception and the error is not being handled by that middleware
    protected $middleware = [
        ......
        /**
         * OAuth 2.0 Middleware
         */
        \LucaDegasperi\OAuth2Server\Middleware\OAuthExceptionHandlerMiddleware::class
    ];

    /**
     * The application's route middleware.
     *
     * @var array
     */
    protected $routeMiddleware = [
        ......

        /**
         * Placed here for OAuth 2.0 configurations
         */
        'csrf'          => App\Http\Middleware\VerifyCsrfToken::class,

        'oauth'         => \LucaDegasperi\OAuth2Server\Middleware\OAuthMiddleware::class,
        'oauth-user'    => \LucaDegasperi\OAuth2Server\Middleware\OAuthUserOwnerMiddleware::class,
        'oauth-client'  => \LucaDegasperi\OAuth2Server\Middleware\OAuthClientOwnerMiddleware::class,
        'check-authorization-params' => \LucaDegasperi\OAuth2Server\Middleware\CheckAuthCodeRequestMiddleware::class,

    ];

So, why is Laravel not answering me a json response? Do you have any idea on which could be the error?

What other middlewares are you using?

Entire middlewares:

    /**
     * The application's global HTTP middleware stack.
     *
     * @var array
     */
    protected $middleware = [
        \Illuminate\Foundation\Http\Middleware\CheckForMaintenanceMode::class,
        \HomeJab\Http\Middleware\EncryptCookies::class,
        \Illuminate\Cookie\Middleware\AddQueuedCookiesToResponse::class,
        \Illuminate\Session\Middleware\StartSession::class,
        \Illuminate\View\Middleware\ShareErrorsFromSession::class,

        /**
         * Commented by OAuth 2.0 configurations
         */
        //\HomeJab\Http\Middleware\VerifyCsrfToken::class,

        /**
         * OAuth 2.0 Middleware
         */
        \LucaDegasperi\OAuth2Server\Middleware\OAuthExceptionHandlerMiddleware::class
    ];

    /**
     * The application's route middleware.
     *
     * @var array
     */
    protected $routeMiddleware = [
        'auth'          => \HomeJab\Http\Middleware\Authenticate::class,
        'auth.basic'    => \Illuminate\Auth\Middleware\AuthenticateWithBasicAuth::class,
        'guest'         => \HomeJab\Http\Middleware\RedirectIfAuthenticated::class,

        /**
         * Placed here for OAuth 2.0 configurations
         */
        'csrf'          => App\Http\Middleware\VerifyCsrfToken::class,

        'oauth'         => \LucaDegasperi\OAuth2Server\Middleware\OAuthMiddleware::class,
        'oauth-user'    => \LucaDegasperi\OAuth2Server\Middleware\OAuthUserOwnerMiddleware::class,
        'oauth-client'  => \LucaDegasperi\OAuth2Server\Middleware\OAuthClientOwnerMiddleware::class,
        'check-authorization-params' => \LucaDegasperi\OAuth2Server\Middleware\CheckAuthCodeRequestMiddleware::class,

        /**
         * Account verified middleware
         */
        'user.verified' => \HomeJab\Http\Middleware\AccountVerifiedMiddleware::class
    ];

If it helps this is the thrown:
AccessDeniedException in ResourceServer.php line 118:
The resource owner or authorization server denied the request.
.....

What middlewares do you use on the specific endpoint?

Route::group(['middleware' => 'cors'], function() {

  .....
  Route::group(['middleware' => 'oauth'], function() {

     .....
      Route::group(['middleware' => 'user.verified'], function() {

which is what you suspect?

Yeah, it probably is the cors that are causing the issue. Please see https://github.com/lucadegasperi/oauth2-server-laravel/issues/614#issuecomment-163945194

It kind works...
I do not know why but have to change code:
        if (($e instanceof OAuthException) || (get_class($e) == "League\OAuth2\Server\Exception\AccessDeniedException")) {
            return response()->json([
                    'error' => $e->errorType,
                    'error_description' => $e->getMessage()
                ], $e->httpStatusCode, $e->getHttpHeaders());
        } else {
            return parent::render($request, $e);
        }

$e instanceof OAuthException gives false.  AccessDeniedException extends from OAuthException


feat: 2FA is now available in all plans

Changelog: Title
Ticket: MEN-5677
Signed-off-by: Fabio Tranchitella<EMAIL_ADDRESS>

@merlin-northern ping

@merlin-northern ping

PONG
yes I am here. sorry for the delay. on it.


Note to reviewer

There are 4 people in this team:
@stephenkati, @Yacoubou-seidou, @Timbar09, @Naledi-Dikgale.
The link to the Kanban board
In this project, we are going to build the Final Capstone Project based on an app for reserving consoles.

[ ] Kanban board screenshot below.

Hi @Naledi-Dikgale,
Wow, you did it 🎉

Your project is complete!  There is nothing else to say other than... it's time to merge it :shipit:
Congratulations! 🎉
To highlight

All cards have been assigned,  pointed and have descriptions✔

Optional suggestions
Every comment with the [OPTIONAL] prefix won't stop the approval of this PR. However, I strongly recommend you to take them into account as they can make your code better. Some of them were simply missed by the previous reviewer and addressing them will really improve your application.
Cheers and Happy coding!👏👏👏
Feel free to leave any questions or comments in the PR thread if something is not 100% clear.
Please, remember to tag me in your question so I can receive the notification.

As described in the Code reviews limits policy  you have a limited number of reviews per project (check the exact number in your Dashboard). If you think that the code review was not fair, you can request a second opinion using this form.


Fix docker asciicast

Re-recorded the Start a Local Cluster in Docker asciicast with the shell window at the proper size. Also, updated the asciicast instructions in contributing.md.

This change is

http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/864d2229c5ce0b4492c4793fdb8d8846456bcee6/

http://cockroach-docs-review.s3-website-us-east-1.amazonaws.com/b942369130df7eeda50682801f7e5c37845253a9/

LGTM


CICD tests should enable low resource simulation in Driver Verifier & App Verifier

To ensure the code is resilient to low memory situation, CICD tests should be run under low resource simulation.

Is this a dup of #1481?

Dup of #1481


Update CredScan suppressions

Use config-based suppressions for all passwords in the flagged .cs files.

Any idea why the inline suppressions didn't work?

Any idea why the inline suppressions didn't work?

Quoting CredScan: NOTE: It is not enough to remove the secrets and push another change. You must reset your branch according to the instructions.
It looks like it checks each of the pulled commits individually. We'd need to bypass it with a special commit. Or once it's there, we might try go back to inline suppression (the pulled change won't contain any secrets in any commits).

My understanding is that we would have to remove them from history if we actually wanted to remove them. Here we've we're just telling CredScan that it's wrong in these cases.

From what I see code-mirror is happy with this now


Migrate from font-awesome to svg

Related to #217

:tada: This issue has been resolved in version 4.0.0 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:


$tmp function modules

Can not create, open the function modules part of temp package. This is possible for Programs and classes though.

should work for your own objects
Support for objects owned by others was added fairly recently but only works when you search for them in the object search dialog

@virendevi234
I just remembered something: sometimes the object list gets corrupted and this extension is not able to rebuild it.
Refreshing the package contents in se80 usually does the trick

Some recent updates should have fixed this, forgot to comment when I made them