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

Use spread syntax to create shallow clones #2310

Closed
4 tasks done
remcohaszing opened this issue Jun 8, 2023 · 2 comments
Closed
4 tasks done

Use spread syntax to create shallow clones #2310

remcohaszing opened this issue Jun 8, 2023 · 2 comments
Labels
🏡 area/internal This affects the hidden internals 💪 phase/solved Post is done

Comments

@remcohaszing
Copy link
Member

Initial checklist

Problem

MDX generates JavaScript code that uses shallow copies. The ones I am aware of are defining _components and compiling JSX props. E.g.:

const _components = Object.assign(
  {
    p: 'p',
    pre: 'pre',
    code: 'code'
  },
  props.components
)

IMO it’s cleaner to use spread syntax for shallow clones. E.g.:

const _components = {
  p: 'p',
  pre: 'pre',
  code: 'code',
  ...props.components
}

I have some nitpicky arguments, but mostly I just personally like it better.

  1. Spread syntax exists for shallow clones
  2. Object.assign exists for shallow copying objects from one object to another. It’s still an elegant way to polyfill spread syntax for environments that don’t support it.
  3. MDX generates code with uses default a default argument. Support for this is roughly the same as for spread syntax.
  4. Spread syntax produces a slightly flatter AST.
  5. Terser can minify it better by flattening / inlining / deduping objects.

Solution

Replace Object.assign() with spread syntax.

Alternatives

Keep using Object.assign().

@remcohaszing remcohaszing added 💬 type/discussion This is a request for comments 🤞 phase/open Post is being triaged manually labels Jun 8, 2023
@wooorm
Copy link
Member

wooorm commented Jun 8, 2023

sure, PR welcome, the idea was to use code that works in most places.
Note that the assign is also used because there are multiple possible sources, it’s pretty complex if I remember correctly

@wooorm wooorm added 👍 phase/yes Post is accepted and can be worked on and removed 🤞 phase/open Post is being triaged manually 💬 type/discussion This is a request for comments labels Jun 8, 2023
@ChristianMurphy ChristianMurphy added the 🏡 area/internal This affects the hidden internals label Sep 22, 2023
@wooorm
Copy link
Member

wooorm commented Oct 18, 2023

Solved in GH-2328.

@wooorm wooorm closed this as completed Oct 18, 2023
@wooorm wooorm added 💪 phase/solved Post is done and removed 👍 phase/yes Post is accepted and can be worked on labels Oct 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏡 area/internal This affects the hidden internals 💪 phase/solved Post is done
Development

No branches or pull requests

3 participants