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

next-swc shakeExports option does not see vars used as JSX #35734

Closed
1 task done
overlookmotel opened this issue Mar 30, 2022 · 3 comments
Closed
1 task done

next-swc shakeExports option does not see vars used as JSX #35734

overlookmotel opened this issue Mar 30, 2022 · 3 comments
Labels
bug Issue was opened via the bug report template.

Comments

@overlookmotel
Copy link
Contributor

overlookmotel commented Mar 30, 2022

Verify canary release

  • I verified that the issue exists in Next.js canary release

Provide environment information

Operating System:
  Platform: darwin
  Arch: x64
  Version: Darwin Kernel Version 18.7.0: Tue Jun 22 19:37:08 PDT 2021; root:xnu-4903.278.70~1/RELEASE_X86_64
Binaries:
  Node: 16.14.2
  npm: 8.5.5
  Yarn: 1.22.18
  pnpm: N/A
Relevant packages:
  next: 12.1.0
  react: 17.0.2
  react-dom: 17.0.2

PR for failing test based on latest canary: #35735

What browser are you using? (if relevant)

No response

How are you deploying your application? (if relevant)

No response

Describe the Bug

I know that next-swc's shakeExports option is not public, but nonetheless I've found what I think is a bug, so I thought I'd report it.

shakeExports avoids shaking variable declarations in top-level scope where they're used within a function in the ignore list.

However, this breaks if the usage is in a JSX tag.

e.g. input:

function ShouldBeKept() {
  return 'should be kept';
}
let shouldBeKept = 'should be kept';
export default function() {
  return <ShouldBeKept val={shouldBeKept} />;
}

Output with shakeExports: { ignore: ['default'] }:

let shouldBeKept = "should be kept";
export default function() {
    return <ShouldBeKept val={shouldBeKept}/>;
}

ShouldBeKept function has been erroneously shaken out.

My best guess is that the cause is upstream in SWC. Possibly dce expects to be used after JSX->JS transform, so it's not looking for variables used in JSX. That's just a total guess though.

Background on why I'm interested in shakeExports: #35487

Expected Behavior

Please see above.

To Reproduce

PR for failing test: #35735

@overlookmotel overlookmotel added the bug Issue was opened via the bug report template. label Mar 30, 2022
@balazsorban44
Copy link
Member

As explained here #35487 (comment) this API is not public so any usage is not guaranteed to work.

Let's keep the discussion in that thread #35487

@balazsorban44 balazsorban44 closed this as not planned Won't fix, can't repro, duplicate, stale Mar 30, 2022
@overlookmotel
Copy link
Contributor Author

Sorry I didn't explain myself well. I didn't open this issue as a demand that it be fixed!

Regardless if it's a public API or not, I'm aware from the discussion that it's in use internally at Vercel, so I thought a bug report and failing test would be helpful.

It turns out that the bug is indeed upstream in SWC's dce transform, so I've raised an issue there and will submit a fix if my meagre Rust skills allow.

@github-actions
Copy link
Contributor

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Apr 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
bug Issue was opened via the bug report template.
Projects
None yet
Development

No branches or pull requests

2 participants