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

Using ncc to build dist/index causes __dirname to end up in "browser bundle" #784

Closed
sjchmiela opened this issue Feb 14, 2022 · 9 comments · Fixed by #802
Closed

Using ncc to build dist/index causes __dirname to end up in "browser bundle" #784

sjchmiela opened this issue Feb 14, 2022 · 9 comments · Fixed by #802

Comments

@sjchmiela
Copy link

Do you want to request a feature or report a bug?

A bug.

What is the current behavior?

styled-jsx/dist/index/index.js includes reference to __dirname. As far as I can tell it's there because ncc is used to compile dist/index and ncc is designed to support Node.js runtime.

/************************************************************************/
/******/ 	/* webpack/runtime/compat */
/******/ 	
/******/ 	if (typeof __nccwpck_require__ !== 'undefined') __nccwpck_require__.ab = __dirname + "/";
/******/ 	
/************************************************************************/

Maybe it's expected that every JS bundler removes __dirname while bundling for the browser?

Reproduce steps

Download https://registry.npmjs.org/styled-jsx/-/styled-jsx-5.0.0.tgz and open package/dist/index/index.js. Search for __dirname (it's in the line 733).

I can also provide a full project a bundler configured etc., but since it has a couple of moving pieces (ESBuild, Babel) I decided to leave it off. Let me know if it would help!

What is the expected behavior?

Bundle to be used in the browser doesn't use __dirname since it's not available there?

Environment (include versions)

  • Version of styled-jsx (or next.js if it's being used): 5.0.0, no next.js
  • Browser: n/a
  • OS: n/a (macOS)

Did this work in previous versions?

Until #770 index.js was being built with babel.

@huozhi
Copy link
Member

huozhi commented Feb 15, 2022

Hi @sjchmiela thanks for noticing this, I'm curious how are you using styled-jsx in your project. Are you playing it without webpack so that it causes an error? A code example will be super helpful

@sjchmiela
Copy link
Author

Ah, that's a story on its own! 😃 At Glow we're bundling a React project with esbuild while using styled-jsx through a custom esbuild+babel plugin (similar to esbuild-babel-plugin).

I decided to support styled-jsx (despite this custom builder setup) since we have a lot of reusable components already written with styled-jsx in other, regular Next.js projects. I actually succeeded to configure everything (I slapped another esbuild plugin on top of everything which replaces __dirname with paths, similar to this one), but in the end wanted to post this issue in case:

  • this has some side effects I don't know about which may be a reason to look into removing this in styled-jsx (instead of relying on us-consumers to deal with it)
  • someone else encounters a similar problem in the future (for reference).

@wegry
Copy link

wegry commented Mar 16, 2022

@huozhi what this error looks like for us with Webpack 4 building and then running in a Chrome extension

index.js?7164:733 Uncaught (in promise) ReferenceError: __dirname is not defined
    at eval (index.js?7164:733:1)
    at Object.eval (index.js?7164:758:11)
    at eval (index.js:760:30)
    at Object../node_modules/styled-jsx/dist/index/index.js (chunkname.ec7e4660aa5bb1463783.js:20548:1)
    at __webpack_require__ (app.js:85:30)
    at eval (style.js?317d:1:18)
    at Object../node_modules/styled-jsx/style.js (chunkName.ec7e4660aa5bb1463783.js:20555:1)
    at __webpack_require__ (app.js:85:30)
    at eval (textField.tsx:3:74)
    at Module../pathToComponent.tsx (chunkname2.ec7073e772c145515caa.js:1971:1)

We also define

// webpack.config.js
module.exports = {
  ...
  node: {
    __dirname: false
  }
}

@wegry
Copy link

wegry commented Mar 30, 2022

After bumping webpack@4 to [email protected], this issue is no longer present in my project.

@huozhi
Copy link
Member

huozhi commented Mar 30, 2022

@sjchmiela Are you using webpack 5? Not sure if it works for your case

@sjchmiela
Copy link
Author

Thanks for the suggestion! We're using esbuild and we already configured it so it replaces __dirname and __filename with appropriate values when building (as stated in #784 (comment)). 🙂

@huozhi
Copy link
Member

huozhi commented Apr 18, 2022

Will close it for now as webpack 5 should be able to handle this and there's workaround for the original case. Feel free to comment if you have any further issues with it

@huozhi huozhi closed this as completed Apr 18, 2022
@Virgil-N
Copy link

I had the same problem with vite. How to solve it?

@huozhi
Copy link
Member

huozhi commented Sep 2, 2022

[email protected] fixed this issue, there's no nodejs specific variables like __dirname in the client bundle

@huozhi huozhi linked a pull request Sep 2, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants