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

[Bug] dist bundle no longer transpiled to es5 #228

Closed
1 task done
nzjames opened this issue Jan 16, 2023 · 4 comments · Fixed by #329
Closed
1 task done

[Bug] dist bundle no longer transpiled to es5 #228

nzjames opened this issue Jan 16, 2023 · 4 comments · Fixed by #329
Assignees
Labels
bug Something isn't working

Comments

@nzjames
Copy link

nzjames commented Jan 16, 2023

Describe the bug

Between version 2.7.4 and 2.8.0 the dist/ portion of the package went from being transpiled to es5 to including nullish coalescing optional chaining in the output.

Reading the release notes of 2.8.0 it does not read like this was an intentional change in the output. This manifests as a regression blocking builds when using a resolution like ^2.7.4 while not transpiling node_modules.

How To Reproduce

  1. npm install react-live-chat-loader@2.7.4
  2. Inspect node_modules/react-live-chat-loader/dist/providers/chatwoot.js and note the transpiled output starting with "use strict"
  3. npm install react-live-chat-loader@2.8.0
  4. Inspect node_modules/react-live-chat-loader/dist/providers/chatwoot.js and note the optional chaining check on line 17 fisrtScript.parentNode?...

Screenshots

No response

Relevant Log Output

No response

Code of Conduct

  • I agree to follow this project’s Code of Conduct
@nzjames nzjames added the bug Something isn't working label Jan 16, 2023
@nzjames
Copy link
Author

nzjames commented Jan 21, 2023

I built the main branch using node 14 and I found the dist/ and module/ outputs transpiled closer to the output of v2.7.4.

Looking at .babelrc I suspect having the targets set to node

"@babel/preset-env",
{
  "targets": {
    "node": true
  }
}

and having the .tool-versions specify

nodejs 16.15.0

results in the output bundles being transpiled to node 16 which supports optional chaining so this is left in.

I'm not an expert in this space but would changing the targets to something like:

"@babel/preset-env",
{
  "targets": {
    "browsers": "last 2 versions"
  }
}

provide a generated output that is safer to use in browser?

@robmorieson robmorieson self-assigned this Feb 1, 2023
@robmorieson
Copy link
Contributor

Hey @nzjames, thanks for raising this one.

We were able to replicate the bug following the steps provided. It seems like the cause of the issue was likely a bump in the @babel/core dependency, as nothing changed in .tool-versions or .babelrc between 2.7.4 and 2.8.0.

Your solution above looks good to me (thanks for that!), so we’ll shoot to get a patch release with the fix rolled out shortly. Cheers!

@altaywtf
Copy link
Contributor

hey there @robmorieson!

first of all, thank you so much for the great work. we truly appreciate it 🙏

I hate to be that person but, 😭 are there any updates regarding this issue? 😭

we had to manually transpile react-live-chat-loader via our webpack config as a workaround, but that's a bit suboptimal.

@robmorieson
Copy link
Contributor

@altaywtf thanks for the gentle bump — this one did fall off the radar so very much appreciated.

Happy to report that a fix for this issue has now been merged and a patch release will be going out including the fix in the next few days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

3 participants