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

chore(babel-preset-gatsby): fix broken link to optimize-hook-destructuring #27350

Merged

Conversation

agilgur5
Copy link
Contributor

@agilgur5 agilgur5 commented Oct 9, 2020

Description

  • previously it pointed to an actual package, however, there is no such
    package and so this link 404'd
    • looking through the blames, this seems to be a plugin implemented
      within preset-gatsby itself, not as a separate package
      • point to correct location there instead
      • would probably be good to have a separate package for it though!

I stumbled upon babel-preset-gatsby a few weeks ago while looking for how other large projects handle Babel configs to get some ideas for TSDX (particularly jaredpalmer/tsdx#383 (comment) / jaredpalmer/tsdx#634) and found this plugin that I had never heard of, but then hit a 404. Took the time to fix that 404 today!

Documentation

It's a docs PR itself!

Related Issues

#19943 is the original proposal.
#22619 (review) mentions separating it out as a separate package, which I guess was rejected/changed, but the docs were accidentally left as is.

Maybe @developit would consider maintaining this as an independent plugin since it's been implemented in both Next and Gatsby?

- previously it pointed to an actual package, however, there is no such
  package and so this link 404'd
  - looking through the blames, this seems to be a plugin implemented
    within preset-gatsby itself, not as a separate package
    - point to correct location there instead
    - would probably be good to have a separate package for it though!
@gatsbot gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 9, 2020
@developit
Copy link
Contributor

@agilgur5 Depending on a common transform would be better for sure. The Next.js one was almost identical to an existing module I'd co-authored, so I've gone ahead and backported your TS version from Gatsby including the holey elements fix. Once merged, Gatsby can depend on babel-plugin-optimize-hook-destructuring.

sanfilippopablo/babel-plugin-optimize-hook-destructuring#13

@agilgur5
Copy link
Contributor Author

agilgur5 commented Oct 12, 2020

your

Oh it certainly wasn't created by me, this is my first PR to Gatsby and this ended up being a one-liner

Once merged, Gatsby can depend on babel-plugin-optimize-hook-destructuring.

💯 I can make a PR to update Gatsby's config once that's merged.

@developit I'm curious if you've proposed this and the other optimizations in #19943 to other libraries like CRA / babel-preset-react-app? I would think TSDX and microbundle could use the Babel ones as well

@vladar vladar added type: documentation An issue or pull request for improving or updating Gatsby's documentation and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Oct 12, 2020
Copy link
Contributor

@vladar vladar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is better than having a broken link. We can follow up when the original plugin is updated. Thanks 👍

@vladar vladar changed the title fix/docs: broken link to optimize-hook-destructuring chore(babel-preset-gatsby): fix broken link to optimize-hook-destructuring Oct 12, 2020
@vladar vladar merged commit 2ffc154 into gatsbyjs:master Oct 12, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: documentation An issue or pull request for improving or updating Gatsby's documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants