Skip to content
This repository was archived by the owner on Jun 19, 2018. It is now read-only.

RFC: Switch from createElement to the default pragma #22

Closed
DrewML opened this issue Feb 2, 2018 · 7 comments
Closed

RFC: Switch from createElement to the default pragma #22

DrewML opened this issue Feb 2, 2018 · 7 comments

Comments

@DrewML
Copy link
Contributor

DrewML commented Feb 2, 2018

Edit: For those looking to contribute, the following changes need to be made:

  1. Remove pragma here, and Babel will fall back to the default
  2. Change createElement import in each file (ex: import { Component, createElement } from 'react';) to import React from 'react'; or import React, { Component } from 'react';
  3. Submit a PR to Peregrine to make the same updates

Part of our reasoning for not using things like TS is that we didn't want to diverge too much from code people will see in examples.

Right now, we're over-writing Babel's pragma for React.createElement to just createElement. This is going to be surprising for people copy/pasting code from tutorials.

Proposing we go back to the default pragma.

@zetlen
Copy link
Contributor

zetlen commented Feb 2, 2018

We overwrote the pragma because we thought we might have to monkeypatch createElement, right? It wasn't just so we could write import { Component, createElement } from "react"; instead of import React, { Component } from "react", right?

@DrewML
Copy link
Contributor Author

DrewML commented Feb 2, 2018

We overwrote the pragma because we thought we might have to monkeypatch createElement, right

No, that wouldn't have anything to do with it.

@DrewML
Copy link
Contributor Author

DrewML commented Feb 2, 2018

It wasn't just so we could write import { Component, createElement } from "react"; instead of import React, { Component } from "react", right?

I can't speak to that - I didn't set this up initially.

@jimbo
Copy link
Contributor

jimbo commented Feb 2, 2018

In the original concept, you (@zetlen) had indicated we might consider replacing React with another library, such as Preact. While I don't think Andrew or I ever supported that idea, it was on my mind when creating this repo that we might have to change every file with JSX at some point.

Whichever pragma we use has to be imported by every file containing JSX. Whether it's createElement or h, we have to import the function, but we never call it. Because the source doesn't use it, but the transpiled code does, it's sort of a hidden dependency. Lint rules for unused variables actually have to carve out an exception for your JSX pragma.

The problem with import React is that createElement is only one of the methods on the React object. If you remove the dependency on createElement by switching to a library with a different JSX pragma, such as h, you can't safely replace import React with import { h } because you don't know what other methods of React may be in use in that module. Explicitly importing createElement is less ambiguous, easier to lint, and easier to find and replace if necessary.

That was the reasoning behind it, anyway. If we're sticking with React for good, it doesn't make much difference what pragma we use. I think comparing this to TypeScript is apples to oranges; this is minor, TS is major.

@DrewML
Copy link
Contributor Author

DrewML commented Feb 13, 2018

you can't safely replace import React with import { h }

This would never be something we have to do, though. If Preact was used, we would have to use preact-compat, which already maps createElement and the other expected React exports

@DrewML
Copy link
Contributor Author

DrewML commented Feb 13, 2018

image

Can we make a fake movie poster for I, Robot but instead it's you, Zetlen

@PWAStudioBot
Copy link

This issue was moved to magento/pwa-studio#21

@DrewML DrewML closed this as completed Jun 13, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

5 participants