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

fix: Don't ignore options.transforms for buble #5677

Merged
merged 4 commits into from
Oct 12, 2021
Merged

Conversation

ntucker
Copy link
Contributor

@ntucker ntucker commented Oct 10, 2021

Motivation

Customizing transformer is not possible, which means disabling transforms like classes etc don't work. This is clearly a bug as it works in normal buble, and the options is spread in but not the options.transforms.

Additionally, we default to allowing classes and getter/setters as these are very useful features.

Have you read the Contributing Guidelines on pull requests?

yes

Test Plan

<LiveProvider
        transpileOptions={{
          target: { chrome: 71 },
          transforms: {
            classes: false,
            letConst: false,
            getterSetter: false,
            generator: false,
            asyncAwait: false,
            moduleImport: false,
            moduleExport: false,
          },
        }}
      >

Make sure it doesn't break on classes or getter setters

Related PRs

(If this PR adds or changes functionality, please take some time to update the docs at https://github.com/facebook/docusaurus, and link to your PR here.)

@netlify
Copy link

netlify bot commented Oct 10, 2021

✔️ [V2]

🔨 Explore the source changes: 51eb073

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/6165cb7a82b5df0008305149

😎 Browse the preview: https://deploy-preview-5677--docusaurus-2.netlify.app

@github-actions
Copy link

github-actions bot commented Oct 10, 2021

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 76
🟢 Accessibility 98
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-5677--docusaurus-2.netlify.app/

@ntucker
Copy link
Contributor Author

ntucker commented Oct 10, 2021

Can't figure out the prettier failure.... the format follows the rest of the file and the github action doesn't have any debug output.

@Josh-Cena
Copy link
Collaborator

This line clearly exceeds the length of 80 characters

Co-authored-by: Joshua Chen <sidachen2003@gmail.com>
@Josh-Cena
Copy link
Collaborator

Oops, should be:

  return transform(source, {
    ...options,
    transforms: {asyncAwait: false, ...options.transforms},
  });

in the latest Prettier version

@slorber
Copy link
Collaborator

slorber commented Oct 12, 2021

LGTM thanks. I fixed the prettier issue, which unfortunately only show problematic files but no diff

@slorber slorber added the pr: bug fix This PR fixes a bug in a past release. label Oct 12, 2021
@slorber slorber merged commit 4a4f849 into facebook:main Oct 12, 2021
@ntucker
Copy link
Contributor Author

ntucker commented Oct 13, 2021

Would love a release with this :D

@slorber
Copy link
Collaborator

slorber commented Oct 14, 2021

We'll release soon, in the meantime you can use a canary release

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr: bug fix This PR fixes a bug in a past release.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants