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

RFC: Support variable declaration (const/let/var) #1046

Closed
johno opened this issue Apr 30, 2020 · 5 comments
Closed

RFC: Support variable declaration (const/let/var) #1046

johno opened this issue Apr 30, 2020 · 5 comments
Milestone

Comments

@johno
Copy link
Member

johno commented Apr 30, 2020

Currently, users leverage export syntax in order to instantiate local variables for usage in an MDX file. We could introduce const/let/var to better define an MDX document's API by allowing users to not need to overload exports for local variables.

Nothing else would really differ from the export syntax except that we'd read const/let/var directly.

const

const MyComponent = () => <div style={{ padding: 20, backgroundColor: 'tomato' }} />

let

let MyComponent = () => <div style={{ padding: 20, backgroundColor: 'tomato' }} />

var

var MyComponent = () => <div style={{ padding: 20, backgroundColor: 'tomato' }} />
@johno johno added 🦋 type/enhancement This is great to have 💬 type/discussion This is a request for comments 🙆 yes/confirmed This is confirmed and ready to be worked on 💎 v2 Issues related to v2 🗄 area/interface This affects the public interface labels Apr 30, 2020
@wooorm
Copy link
Member

wooorm commented May 1, 2020

I think there are two alternatives (using a different style to show that they’d allow more too):

  • b) executable code
    # heading
    
    ```js run
    function MyComponent() {
      return <div style={{ padding: 20, backgroundColor: 'tomato' }} />
    }
    ```
    
    paragraph
  • c) expressions
    # heading
    
    {
      function MyComponent() {
        return <div style={{ padding: 20, backgroundColor: 'tomato' }} />
      }
    }
    
    paragraph

(note: we could also allow imports/exports in those. And even allow both syntax B && C, where syntax C would be a shortcut for B?)

@johno johno changed the title RFC: Support variable declaration (const/let/var) RFC: Support variable declaration (const/let/var) ✅ May 20, 2020
@johno johno added this to the v2 milestone Jul 22, 2020
wooorm added a commit that referenced this issue Dec 20, 2020
This removes the last three custom Babel plugins we had and replaces
them with estree versions.
Furthermore, it removes `@babel/generator`.

For the plugins, we were only looking at ESM import/exports, but right
now we’re delegating work to `periscopic` to look at which things are
defined in the top-level scope.
It’s a bit more complex, but this matches better with intentions,
fixes some bugs, and prepares for a potential future where other ES
constructs are allowed, so all in all should be a nice improvement.

For serializing, we’re switching to `astring`, and handling JSX for now
internally (could be externalized later).
`astring` seems fast and is incredibly small, but is not very popular.
We might perhaps see bugs is serialization in the future because of that,
but all our tests seem fine, so I’m not too worried about that.

Estree remains a somewhat fragmented ecosystem, such as that the tree
walkers in `periscopic` and `astring` are different, so we might also
consider writing our own serializer in the future.
Or, when we implement Babel’s React JSX transform ourselves, could switch
to another generator, or at least drop the JSX serialization code here.

Because of these changes, we can drop `@babel/core` and
`@babel/generator` from `@mdx-js/mdx`, which drops the bundle size of
from 349kb to 111kb.
That’s 68%.
Pretty nice.
This should improve downloading and parsing time of bundles
significantly.
Of course, we currently still have JSX in the output, so folks will
have to resort to Babel (or `buble-jsx-only`) in another step.

For performance, v2 (micromark) was already an improvement over v1.
On 1000 simple files totalling about 1mb of MDX:

* v1: 3739ms
* v2: 2734ms (26% faster)
* v2 (w/o babel): 1392ms (63% faster).

Of course, this all really depends on what type of stuff is in your MDX.
But it looks pretty sweet!

✨

Related to GH-1046.
Related to GH-1152.
Related to GH-1338.
Closes GH-704.
Closes GH-1384.
johno pushed a commit that referenced this issue Dec 20, 2020
This removes the last three custom Babel plugins we had and replaces
them with estree versions.
Furthermore, it removes `@babel/generator`.

For the plugins, we were only looking at ESM import/exports, but right
now we’re delegating work to `periscopic` to look at which things are
defined in the top-level scope.
It’s a bit more complex, but this matches better with intentions,
fixes some bugs, and prepares for a potential future where other ES
constructs are allowed, so all in all should be a nice improvement.

For serializing, we’re switching to `astring`, and handling JSX for now
internally (could be externalized later).
`astring` seems fast and is incredibly small, but is not very popular.
We might perhaps see bugs is serialization in the future because of that,
but all our tests seem fine, so I’m not too worried about that.

Estree remains a somewhat fragmented ecosystem, such as that the tree
walkers in `periscopic` and `astring` are different, so we might also
consider writing our own serializer in the future.
Or, when we implement Babel’s React JSX transform ourselves, could switch
to another generator, or at least drop the JSX serialization code here.

Because of these changes, we can drop `@babel/core` and
`@babel/generator` from `@mdx-js/mdx`, which drops the bundle size of
from 349kb to 111kb.
That’s 68%.
Pretty nice.
This should improve downloading and parsing time of bundles
significantly.
Of course, we currently still have JSX in the output, so folks will
have to resort to Babel (or `buble-jsx-only`) in another step.

For performance, v2 (micromark) was already an improvement over v1.
On 1000 simple files totalling about 1mb of MDX:

* v1: 3739ms
* v2: 2734ms (26% faster)
* v2 (w/o babel): 1392ms (63% faster).

Of course, this all really depends on what type of stuff is in your MDX.
But it looks pretty sweet!

✨

Related to GH-1046.
Related to GH-1152.
Related to GH-1338.
Closes GH-704.
Closes GH-1384.
@aleclarson
Copy link

This feature would be much appreciated, as export const breaks Fast Refresh when its value is not a component.

@ng-hai

This comment was marked as off-topic.

@wooorm

This comment was marked as resolved.

@wooorm wooorm removed the 💎 v2 Issues related to v2 label Mar 31, 2022
@wooorm wooorm changed the title RFC: Support variable declaration (const/let/var) ✅ RFC: Support variable declaration (const/let/var) Mar 31, 2022
@wooorm
Copy link
Member

wooorm commented Oct 2, 2024

Closing as there is little traction on this.
The main problem I see is that we need to make exceptions for more and more words. And what about function MyComponent, for example?
There is a good alternative possible, as outlined above: ~~~js eval. That can be supported in a plugin. The pseudocode for that is in in this comment.

@wooorm wooorm closed this as not planned Won't fix, can't repro, duplicate, stale Oct 2, 2024
@wooorm wooorm removed 🦋 type/enhancement This is great to have 💬 type/discussion This is a request for comments 🙆 yes/confirmed This is confirmed and ready to be worked on 🗄 area/interface This affects the public interface labels Oct 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

4 participants