4 ms·

My biggest concern is separating the concerns via file naming convention and relying on presumably linter plugins to enforce its rules and on modifications to p

My biggest concern is separating the concerns via file naming convention and relying on presumably linter plugins to enforce its rules and on modifications to packagers, rather than something that can be used more directly in a type system and can more directly resemble ordinary tree-shaking. I don't have a strong counter-proposal off the top of my head, but I do think the RFC is too quick to dismiss async/await and dynamic imports. async/await makes a lot of sense to differentiate server-side components (Promise<T> returns would be a very different type of return from current components), many things you would use server like DB access would want you to use Promises today. (That seems a clear problem with the server side examples as presented in the RFC: they use synchronous node 'fs' instead of FS Promises, and some sort of synchronous 'db' that looks less and less like any modern Node db provider.) There's even a possible intuition pump there that hooks "obviously" won't work in an async function making it easier to keep the rules in mind between the types of components. If the components are then async/await by "requirement", dynamic imports stop looking so out of place and start to look much more correct in the component body.

acemarke 6y ago
Probably worth leaving this feedback on the RFC itself, then :)
- WorldMaker 6y ago
  This feedback is me working out loud the problem if I have solutions to offer the RFC itself, but I would happily apply that feedback to the RFC itself if I did feel like I made headway toward the solution.
danabramov 6y ago
>That seems a clear problem with the server side examples as presented in the RFC: they use synchronous node 'fs' instead of FS Promises It isn't synchronous, it only looks so. It actually uses FP Promises under the hood[1]. We're doing a bit of a trick there (it throws a special value the first time, and then React retries when it's resolved). Then the result gets cached, and the repeated render succeeds synchronously. We will describe this mechanism in detail in a future RFC. All of the examples in the demo (including fetch, readFile, and the DB query call) are async under the hood. However, they do leave us the ability to use sync I/O in the cases where it makes sense (depends on whether we have other work to do, etc). [1]: https://github.com/facebook/react/blob/6cbb9394d1474e3a728b49dc7f3a11d61a421ce3/packages/react-fs/src/ReactFilesystem.js#L13 https://github.com/facebook/react/blob/6cbb9394d1474e3a728b4...
- dfabulich 6y ago
  Yes, and that's a problem: there's no way to know when you call react-fs/react-fetch/etc whether it will be synchronous or asynchronous. async/await would make this code substantially clearer. (To be clear, right now the code is using a, uh, very surprising pattern to make asynchronous code appear synchronous: if the result value is cached, it returns the value synchronously, but if not, the fetcher throws a Promise. You know, you'd normally throw exception objects, but JS lets you throw any value, so why not throw a Promise amirite?!? When React catches a Promise (a thenable) it awaits the result, caches it and then re-runs the React component; now the component won't throw a Promise and will run to completion normally.) The FAQ says extremely little about why async/await were avoided: > Why don’t use just use async/await? > We’d still need a layer on top, for example, to deduplicate fetches between components within a single request. This is why there are wrappers around async APIs. You will be able to write your own. We also want to avoid delays in the case that data is synchronously available -- note that async/await uses Promises and incurs an extra tick in these cases. A layer to deduplicate fetches sounds great, but that library could use async/await, too. Using async/await will make this code substantially easier to understand, and the cost of a "tick" is trivial (and certainly worth the price, particularly in server-side code). EDIT: Thinking about this a bit harder, I know the React team has been extremely resistant to async/await in components for years now. Fine. But that needs its own RFC. Some clear written document spelling out in detail why async/await is the wrong approach for React, and not just a comment on this RFC. I'd like to ask the React team to write that RFC doc because I think it can't be written: you'll find that the argument falls apart when you try to explain it.
  - danabramov 6y ago
    We will definitely do the RFC in the future. Challenge accepted. :-)
    - no_wizard 6y ago
      I know this is old now by a day but to add on, Is it possible you could document why then react doesn’t handle plan promises natively? You can conditionally return a promise there by not incurring the cost of the sugar of async/await Also related: why wouldn’t generators be a good fit for some of these use cases? Especially since iterating over data is a common action. I imagine this would be easier to integrate into the framework for things like this
dfabulich 6y ago
Surprisingly, their implementation of `react-fs` doesn't use synchronous node `fs.readFileSync`… instead, they're doing some trickery (throwing a Promise!) to make `react-fs` appear synchronous when it's really asynchronous! https://github.com/facebook/react/blob/master/packages/react-fs/src/ReactFilesystem.js https://github.com/facebook/react/blob/master/packages/react... https://github.com/facebook/react/blob/master/packages/react-cache/src/ReactCacheOld.js https://github.com/facebook/react/blob/master/packages/react... However, I agree that using async/await would be better.
sugarpirate 6y ago
There is also another RFC that proposes alternatives: https://github.com/reactjs/rfcs/pull/189 https://github.com/reactjs/rfcs/pull/189

acemarke 6y ago

Probably worth leaving this feedback on the RFC itself, then :)

WorldMaker 6y ago
This feedback is me working out loud the problem if I have solutions to offer the RFC itself, but I would happily apply that feedback to the RFC itself if I did feel like I made headway toward the solution.

danabramov 6y ago

>That seems a clear problem with the server side examples as presented in the RFC: they use synchronous node 'fs' instead of FS Promises It isn't synchronous, it only looks so. It actually uses FP Promises under the hood[1]. We're doing a bit of a trick there (it throws a special value the first time, and then React retries when it's resolved). Then the result gets cached, and the repeated render succeeds synchronously. We will describe this mechanism in detail in a future RFC. All of the examples in the demo (including fetch, readFile, and the DB query call) are async under the hood. However, they do leave us the ability to use sync I/O in the cases where it makes sense (depends on whether we have other work to do, etc). [1]: https://github.com/facebook/react/blob/6cbb9394d1474e3a728b49dc7f3a11d61a421ce3/packages/react-fs/src/ReactFilesystem.js#L13 https://github.com/facebook/react/blob/6cbb9394d1474e3a728b4...

dfabulich 6y ago
Yes, and that's a problem: there's no way to know when you call react-fs/react-fetch/etc whether it will be synchronous or asynchronous. async/await would make this code substantially clearer. (To be clear, right now the code is using a, uh, very surprising pattern to make asynchronous code appear synchronous: if the result value is cached, it returns the value synchronously, but if not, the fetcher throws a Promise. You know, you'd normally throw exception objects, but JS lets you throw any value, so why not throw a Promise amirite?!? When React catches a Promise (a thenable) it awaits the result, caches it and then re-runs the React component; now the component won't throw a Promise and will run to completion normally.) The FAQ says extremely little about why async/await were avoided: > Why don’t use just use async/await? > We’d still need a layer on top, for example, to deduplicate fetches between components within a single request. This is why there are wrappers around async APIs. You will be able to write your own. We also want to avoid delays in the case that data is synchronously available -- note that async/await uses Promises and incurs an extra tick in these cases. A layer to deduplicate fetches sounds great, but that library could use async/await, too. Using async/await will make this code substantially easier to understand, and the cost of a "tick" is trivial (and certainly worth the price, particularly in server-side code). EDIT: Thinking about this a bit harder, I know the React team has been extremely resistant to async/await in components for years now. Fine. But that needs its own RFC. Some clear written document spelling out in detail why async/await is the wrong approach for React, and not just a comment on this RFC. I'd like to ask the React team to write that RFC doc because I think it can't be written: you'll find that the argument falls apart when you try to explain it.
- danabramov 6y ago
  We will definitely do the RFC in the future. Challenge accepted. :-)
  - no_wizard 6y ago
    I know this is old now by a day but to add on, Is it possible you could document why then react doesn’t handle plan promises natively? You can conditionally return a promise there by not incurring the cost of the sugar of async/await Also related: why wouldn’t generators be a good fit for some of these use cases? Especially since iterating over data is a common action. I imagine this would be easier to integrate into the framework for things like this

dfabulich 6y ago

Surprisingly, their implementation of `react-fs` doesn't use synchronous node `fs.readFileSync`… instead, they're doing some trickery (throwing a Promise!) to make `react-fs` appear synchronous when it's really asynchronous! https://github.com/facebook/react/blob/master/packages/react-fs/src/ReactFilesystem.js https://github.com/facebook/react/blob/master/packages/react... https://github.com/facebook/react/blob/master/packages/react-cache/src/ReactCacheOld.js https://github.com/facebook/react/blob/master/packages/react... However, I agree that using async/await would be better.

sugarpirate 6y ago

There is also another RFC that proposes alternatives: https://github.com/reactjs/rfcs/pull/189 https://github.com/reactjs/rfcs/pull/189