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

Suggestion to mention about ESBUILD_BINARY_PATH env var in README.md. #15

Closed
grzegorzpokorski opened this issue Jul 23, 2022 · 10 comments
Closed

Comments

@grzegorzpokorski
Copy link
Contributor

grzegorzpokorski commented Jul 23, 2022

I think t would be worthy to mention in README.md that if someone want to run the project on windows platform (or different) in local environment, should check if have a correct path to executable file (esbuild.exe or esbuild) in ./helpers/content/mdx.ts.

Bad path to esbuild executable make impossible to compile and bundle mdx files.

Something like that would be enough: "Take care about correct ESBUILD_BINARY_PATH env variable which is defined in ./helpers/content/mdx.ts file".

For example for me I work on windows 10 x64 and correct path to esbuild.exe is: ./node_modules/esbuild-windows-64/esbuild.exe.

@robert-orlinski
Copy link
Owner

robert-orlinski commented Aug 7, 2022

Good point @grzegorzpokorski. I'm not using Windows so I just copied code from mdx-bundler's readme without testing it 😅

I am only wondering, because we have separate esbuild package for every platform Node recognises, shouldn't just installing esbuild on Windows work instead of using specific package for windows?

Maybe you can install just esbuild and compile MDX using it instead of esbuild-windows-64? May you let me know about it?

If you will be able to, I think we can add just some information that if your error happen, we need to install main esbuild instead of specific platform's build.

@grzegorzpokorski
Copy link
Contributor Author

Hi @robert-orlinski. I only cloned your repo and run npm install. I was little bit confused because in package.json is only esbuild package defined. Package named esbuild-windows-64 for my enviroment has been downloaded by esbuild package itself.

If I good understand, esbuild package downloads additional necessary package depending on OS and in this additional package is located executable file for esbuild for specyfic platform. So if i good think there isn't universal path to executable file of esbuild for all platforms.

@robert-orlinski
Copy link
Owner

robert-orlinski commented Aug 8, 2022

You are right @grzegorzpokorski, as I see, also on Mac, there is eslint and additional esbuild-darwin-64 installed (counterpart for esbuild-windows-64).

But this is weird that on Mac environment esbuild app which is run by mdx-bundler is placed in esbuild catalogue while on Windows it's placed in esbuild-windows-64 as you mentioned.

I will get access to Windows environment and test it 🙂 Maybe I will find something interesting!

@robert-orlinski
Copy link
Owner

To be sure - there is no executable in esbuild folder in your environment? There is only one in esbuild-windows-64, yes?

@grzegorzpokorski
Copy link
Contributor Author

Hi! Below you can see folders related to esbuild which appear in node_modules folder after run npm install command:

Zrzut ekranu 2022-08-08 234839

@grzegorzpokorski
Copy link
Contributor Author

After run app by npm run dev and navigate to any article (e.g http://localhost:3000/artykuly/nowa-wersja-robertorlinski.pl) ENOENT error apears like on screenshot below:

Zrzut ekranu 2022-08-08 235511

@grzegorzpokorski
Copy link
Contributor Author

grzegorzpokorski commented Aug 8, 2022

I've just found that following path to executable file of esbuild also work correctly on my environment:

process.env.ESBUILD_BINARY_PATH = path.join(
  process.cwd(),
  'node_modules',
  '.bin',
  'esbuild',
);

... and if i good understand this npm documentation article, above path could be used as 'universal' for win32 platforms 🤔

@robert-orlinski
Copy link
Owner

robert-orlinski commented Aug 10, 2022

@grzegorzpokorski WOW, you are right! It's awesome that this change works also for Macs as I see!

If we go even further, what about deleting ESBUILD_BINARY_PATH env variable overwritting in general? Will it work on your environment? If yes, I will check if deployment will work with this deletion.

I am asking because this if related to ESBUILD_BINARY_PATH variable, is here because of this article's part in which Adam is mentioning that Next.js and mdx-bundler don't work together without this if and as I remember, this problem was occurring when I was playing with this code for the first time.

Now it disappeared (at least on my environment) so I can expect that this is no longer the case - maybe there was some Next.js or mdx-bundler update that solved it 🤔

@grzegorzpokorski
Copy link
Contributor Author

Yeah, you can get rid of whole if statement 😂 On my environment - windows 10 x64 - mdx-bundler works correctly also without declaring ESBUILD_BINARY_PATH variable 🙂

Below more information why ESBUILD_BINARY_PATH variable is no longer needed:

@robert-orlinski
Copy link
Owner

Hi @grzegorzpokorski!

I am sorry for this late reply. I haven't noticed any notification regarding your last comment 😮

I removed this if then! Thank you for additional links, opening this issue, and the whole conversation 🥳

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

No branches or pull requests

2 participants