Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding compilation solutions to improve developer experience #141

Closed
wants to merge 5 commits into from

Conversation

carpben
Copy link

@carpben carpben commented Jan 18, 2020

Recently I was introduced to Svelte, and was impressed by the ability to write less implementation details, less source code, and easily use reactive values. However, I wondered if to achieve such benefits, a new higher level language was really necessary. For me, the ability to use JS freely is valuable. I wonder, can we significantly improve developer experience in such a way and remain in JS?

From a different angle, certain aspects of programming in modern React require technical, verbose, and in certain cases trivial code. Such is the case when trying to update a nested state value immutably, or when passing dependencies to hooks.

This RFC is about adding compilation solutions to improve developer experience. To achieve this it considers:

  1. Adding new syntactic features
  2. As an alternative, leveraging Babble Macros for this purpose.

View formatted RFC

@phryneas
Copy link

I have my concerns. Not only would this require adoption from React, but also adoption from the complete ecosystem, meaning TypeScript, Flow, Prettier, Eslint etc. Which means it would take a lot of effort to implement.
Of course, it could be worth it. But in my opinion it isn't. Let me elaborate:

  • Creating a shorthand syntax for useCallback and useMemo will increase their usage.
    This is most likely a bad thing™, as using these does not come without cost and does not always provide a benefit. In the given example, only memoizing completedTasks might bring a benefit, memoizing the callbacks is most probably neglectable and will probably even add a memory overhead. The only place where I believe this is useful is when creating callbacks that will be passed to child components wrapped in memo - which, without good reason and real-life performance testing is another anti-pattern.
  • Creating a shorthand syntax for useState that hides modifications might introduce a whole new class of bugs. As these values have to be assignable, they can neither be marked as const nor will it be possible to use future JavaScript proposals like records & tuples that would guarantee deep immutability.
    Instead, accidental assignments will rise - they might now trigger a re-render, but I wouldn't call that a win.
    Also, this will have runtime overhead and bundle size increase as right now, using immer (or similar) is optional where required and would then become default behaviour. Why not just use a custom hook useStateWithImmer that wraps the callback to setState with produce?
  • The example for shorthand attribute assignment could already be wrtitten <FancyButton {...{handler, mode, size, text}} /> which would not need a new syntax that further deviates from standard JS

Also, as all this would require an additional transpilation step and thus modifications in every project's build-chain, it would definitely be a breaking change.

@carpben
Copy link
Author

carpben commented Jan 21, 2020

Creating a shorthand syntax for useCallback and useMemo will increase their usage. This is most likely a bad thing™

I agree that we shouldn't be attached to optimizing, but we shouldn't be attached to non-optimizing as well. Memoizing should be weighed for it's benefit and cost. As for the benefits, it really depends on the UI you are building. In general, the more dynamic and higher in the UI tree a component is, there will be a bigger benefit from avoiding unnecessary re-rendering. As for the cost of memoizing in React, there are two which are important. One - It's a burden to the developer and to the code. Two - if the a nested mutable prop is passed to a pure component (or to a memoized component) the component won't re-render when it should. Suggestions in this RFC address both. It leaves one question open - when it makes sense to memoize. This is where testing and developer tools come into play. But some rules of thumbs can be helpful as well. While memoizing should definitely not be done by default, it's also not a rare to use tool in React.

As these values have to be assignable, they can neither be marked as const

It will be a new syntax which initially Typescript won't support. I agree it's really expensive.

Why not just use a custom hook useStateWithImmer that wraps the callback to setState with produce?

There is such a hook, published by Immer. However, there are certain things only compilation can do. Passing state (or draft) each time as a parameter is trivial, and I believe trivial code could be avoided with compilation.

The example for shorthand attribute assignment could already be wrtitten <FancyButton {...{handler, mode, size, text}} />

There is a 6 year long discussion about the feature, and why <FancyButton {...{handler, mode, size, text}} /> doesn't solve the issue (facebook/jsx#23).

it would definitely be a breaking change.

AFAIK breaking change means that updating a project to a newer version of a library might break it and introduce bugs. Adding syntax will require a dedicated new file extension, and the transformations will only take place on those files, so will not affect older source code.

@carpben
Copy link
Author

carpben commented Jan 21, 2020

This feature request was inspired by Svelte. While personally I'd love to see new types of values/definitions as suggested in this RFC, as a community there are other things to consider. @phryneas is right that the cost of development for a new syntactic feature is very expensive, and therefor, might just not be worth it.

However, the issue presented in this RFC is valid, and what the RFC aims for is important. However, solution might not come from new syntax, but from existing syntax - from JS to JS transformations. certain solutions were already suggested and discussed (issue1 issue2). An overall approach can yield most of the benefits presented in this RFC, with a minimal cost.

Babel macros alternative

  1. One memoizing macro - It will simplify memoization in React, and will appropriately transform into React.memo, useMemo or useCallback, and pass dependencies.
  2. A macro which simplifies state management.
import {useStateM, $} from "react/macros"

const CompleteAndShare = ({contactsCompletedStats, contacts}) => {
	const [tasks, setTasks] = useStateM(initialTasks)

	const toggleCompleted = $( (id) => setTasks(tasks[id].completed = !tasks[id].completed ) )
	
	const editSubTask = $( (taskId, subTaskId, newText) => setTasks(tasks[taskId][subTaskId].text = newText ))

	const completedTasks = $( tasks.reduce(
		(accum, task) => accum+task.completed,
		0
	))

	const shareCompletedTasks = $( (contactId) => API.shareCompleted(contactId, completedTasks) )

	return (
		<>
			<h1>Complete and share</h1>
			<ControlPanel shareCompletedTasks={shareCompletedTasks} contacts={contacts} />

			<ContactStats contactsCompletedTasks={contactsCompletedTasks} />
			<SelfStats completedTasks={completedTasks} />
			
			<TaskList tasks={tasks} toggleCompleted={toggleCompleted} editSubTask={editSubTask} />
		</>
	)
})

export default $(CompleteAndShare)

@phryneas
Copy link

const editSubTask = $( (taskId, subTaskId, newText) => setTasks(tasks[taskId][subTaskId].text = newText ))

This syntax is extremely dangerous, as without a babel transform it would be an assignment in a function argument position. People new to JavaScript might pick this up, accidentally use it outside of a babel transform and modify their state.

Honestly: please do not try to invent new syntax for JavaScript.
TypeScript has done that with enums, namespaces and decorators and to this day, this makes it very difficult to transpile (const enums and namespaces with babel-preset-typescript just don't work) and even collided with current JavaScript proposals (see decorators).

If you want to change the language to support assignment with copied return value on immutable data structures, a TC39 proposal would be the only correct way, not going through the React-backdoor. This might be worth discussing in the records & tuples proposal I already linked above

@carpben
Copy link
Author

carpben commented Jan 25, 2020

const editSubTask = $( (taskId, subTaskId, newText) => setTasks(tasks[taskId][subTaskId].text = newText ))

... Honestly: please do not try to invent new syntax for JavaScript.

The code you are quoting and referring to isn't new syntax.

As for your concern that a user might not be aware of the transformations that is taking place by by a Babel macro, this is a general concern about macros. One solution is to be more explicit about the macro.

More importantly, this RFC and the code examples, as clearly mentioned in the RFC, were not intended to be ready for implementation. On the contrary. The purpose was to start a discussion about the need for compilation solutions to improve developer experience in general, and more specifically in the context of memoization and state management. As evident by your first comment, this is a purpose you don't support. But it is a feature that others in the community are looking for and that React team to some extend considers.

I respect your concerns, some of them are important. I also respect your opinion that there is no need for such compilation solutions. But to justify your opinion, you go into details, which are not the essence of the RFC at this stage.

I'd love for a wider discussion to take place.

@carpben carpben changed the title Adding syntactic features Adding compilation solutions to improve developer experience Jan 25, 2020
to allow for a more focused discussion.
Current discussions refers only to adding syntactic features. But the essence of this RFC is improving developer experience with compilation solutions.
@carpben
Copy link
Author

carpben commented Sep 28, 2020

Personally I’m inspired by new higher order languages such as Svelte and Reason, which allow us to write succinct code which compiles to Javascript.

Through this RFC I was hoping to allow for more succinct coding/syntax in React. However, the RFC was not well received. It seems readers thought we should not divert from JS syntax. JSX in this aspect is the exception.

Originally I hoped that the RFC could start a productive active discussion, but this didn’t happen. 8 month later it’s time to close the RFC.

@carpben carpben closed this Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants