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

Describe current Site integration issues with CRA 5 in documentation #2004

Closed
4 tasks done
userzimmermann opened this issue Apr 8, 2022 · 12 comments
Closed
4 tasks done
Labels
📚 area/docs This affects documentation 🕸 area/website This affects the website 💪 phase/solved Post is done

Comments

@userzimmermann
Copy link
Contributor

Initial checklist

Problem

https://mdxjs.com/docs/getting-started/#create-react-app-cra states that: "There is no need to configure it."

Unfortunately, that's currently not the case with CRA 5 anymore; and many people (including myself) have already spent a lot of time with figuring out reasons & finding workarounds …

Solution

Please add a statement to https://mdxjs.com/docs/getting-started/#create-react-app-cra —briefly describing the situation & maybe directing people to possible workarounds like those discussed in #1870

Alternatives

Just accepting the situation and waiting for facebook/create-react-app#12166 to get fixed …

@wooorm
Copy link
Member

wooorm commented Apr 8, 2022

Sounds like you have the insights needed to document this? Are you perhaps interested in contributing?

@wooorm wooorm added help wanted 🙏 This could use your insight or help good first issue 👋 This may be a great place to get started! 📚 area/docs This affects documentation 👍 phase/yes Post is accepted and can be worked on labels Apr 8, 2022
@userzimmermann
Copy link
Contributor Author

Ok, @wooorm – If you direct me to the corresponding documentation sources, then I'll add it :)

@wooorm
Copy link
Member

wooorm commented Apr 8, 2022

At the bottom of each page on the website is a link “Edit this page on GitHub”, that points you to the file you want to edit. More on how to contribute is explained in the contribution guide: https://mdxjs.com/community/contribute/.

@userzimmermann
Copy link
Contributor Author

Good morning, @wooorm :) & first: Thanks for quickly merging #2007 —yet I still can't build the documentation locally:

$ npm run docs-generate

> docs-generate
> cross-env NODE_LOADER_CONFIG=website/loader-react-server.js node --unhandled-rejections=strict --no-warnings --experimental-loader @node-loader/core --conditions=react-server website/generate.server.js

file:///C:/Users/userz/Projects/mdx/docs/_component/layout.server.js:56
      <div>
      ^

SyntaxError: Unexpected token '<'
    at ESMLoader.moduleStrategy (node:internal/modules/esm/translators:139:18)
    at ESMLoader.moduleProvider (node:internal/modules/esm/loader:236:14)

JSX syntax is not accepted in .js files; which sounds reasonable ;) —but somehow it must work for you (and your CI) … Am I missing some Node.js environment setting to enable this general JSX support? Is this Windows-related?

And—a maybe blasphemous question ;D —why not using .jsx file endings?

@wooorm
Copy link
Member

wooorm commented Apr 9, 2022

Use Node 14.

Unfortunately Vercel only supports Node 14 currently.
Experimental node loaders changed between v14 and v17+, but we can’t update to the fix because Vercel is stuck on an old Node version.

why not using .jsx file endings?

That would not solve the problem here. The problem isn’t which files are loaded. The problem is that custom loaders (e.g., JSX or MDX -> JS) don’t work.

@userzimmermann
Copy link
Contributor Author

Use Node 14.

Which wonderfully works!—& as a side-effect I'm now familiar with NVM!—& #2009 tears down my final contribution barrier :)

@wooorm wooorm closed this as completed in 7f9a5f4 May 9, 2022
@wooorm wooorm added 💪 phase/solved Post is done 🕸 area/website This affects the website and removed help wanted 🙏 This could use your insight or help good first issue 👋 This may be a great place to get started! 👍 phase/yes Post is accepted and can be worked on labels May 9, 2022
@echarles
Copy link

echarles commented Apr 1, 2023

I hit the same issue while generating the docs site with node 18 and indeed node 14 solves it.

  1. Should an engine key be added to package.json to inform developers of that constraint?
  2. Does that mean that the whole MDX library ecosystem do no work with node >14?

@wooorm
Copy link
Member

wooorm commented Apr 1, 2023

Are you talking about CRA, per this issue, or about the docs of this project?

Re 1: No, this is about the docs site
Re 2: No, MDX can as far as I am aware even be used on Node 10, with some experimental esm flag

@echarles
Copy link

echarles commented Apr 1, 2023

Are you talking about CRA, per this issue, or about the docs of this project?

talking about the docs with npm run docs - Sorry I just realize that the issue is about CRA, so may not be the best place to discuss...

Re 1: No, this is about the docs site

yep, so if the docs can only be built with Node 14, engines key could be added https://docs.npmjs.com/cli/v9/configuring-npm/package-json#engines

Re 2: No, MDX can as far as I am aware even be used on Node 10, with some experimental esm flag

Is that documented somewhere?

@remcohaszing
Copy link
Member

yep, so if the docs can only be built with Node 14, engines key could be added https://docs.npmjs.com/cli/v9/configuring-npm/package-json#engines

Adding engines would cause npm install to fail for the mono repo for other Node.js versions.

Is that documented somewhere?

Yes, it’s docmented for every package, e.g.: https://mdxjs.com/packages/mdx/#install

@wooorm
Copy link
Member

wooorm commented Apr 1, 2023

For how to build the site, see the contribution docs: https://mdxjs.com/community/contribute/#site

Is that documented somewhere?

The recommendation in the docs is to use a maintained version of Node. That is Node 14 currently. End of the month that will be Node 16: https://github.com/nodejs/Release.
All docs currently mention 12, because Node 10 needed a flag, and you should be at 14+ anyway.

@echarles
Copy link

echarles commented Apr 1, 2023

Agree, engines is not the way to go as would break other modules. https://mdxjs.com/community/contribute/#site is informative, maybe a specific script to check the node/npm version would have helped as a developer.

Screenshot 2023-04-01 at 11 55 21

Thx also for the infos about current and future support of node.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📚 area/docs This affects documentation 🕸 area/website This affects the website 💪 phase/solved Post is done
Development

No branches or pull requests

4 participants