I think we should fix this in Gutenberg directly. We should make sure that it works by default without setting this rather than having npm consumers learn about this.

yeah, that makes sense! I see that this is actually a GB issue because it should have been replaced at build time and we even have a lint for it.

Do you have any idea why that would not be the case?

I see that the version of the packages published to npm does not have the value of process.env.IS_GUTENBERG_PLUGIN replaced. E.g.: You can check here: https://unpkg.com/browse/@wordpress/core-data@6.31.0/build/resolvers.js

@michalczaplinski yes, that's fine, because Gutenberg also consumes the built npm packages like any other app. The problem is that it shouldn't cause an error by default but should assume "false" if you don't define the variable.

This is fine but I kind of thought that vite uses the automatic JSX config by default. it's not the case?

There are more details in https://v2.vitejs.dev/guide/features.html#jsx but TL;DR is that it's not the case out of the box. We can also do one of those things

• use the React plugin https://www.npmjs.com/package/@vitejs/plugin-react
• add sth like those lines to the vite config:

  export default defineConfig({
   esbuild: {
       jsxInject: `import React from 'react'`
     }
  })

Let's keep it simple and just import React like you did for now.

If you create the Vite app with the "React" / "JavaScript" config instead of "Vanilla" / "JavaScript" config, this works automatically. The vite config becomes:

import { defineConfig } from 'vite'
import react from '@vitejs/plugin-react'

// https://vitejs.dev/config/
export default defineConfig({
  plugins: [react()],
});

Actually, starting with the "Vanilla" vite config cerates a few issues (jsx, react), why not change the docs to suggeste starting with the "React" vite config? Those things will be solved.

If starting with "Vanilla" then react and react-dom aren’t added as dependencies so it seems like that would warrant mentioning them in "Installing dependencies" section.

I'd favor instructing to use the "React" variant. It seems like it simplifies the instructions and provides a better starting place as it adds some helpful packages like eslint with react-related plugins.

I actually don't think we should add this to the docs, it doesn't make sense for third-party developers and shouldn't be required.

I looked a bit into this and found that a similar request for the @wordpress/warning package was addressed by creating a SCRIPT_DEBUG global. There was a conversation about an alternative approach that was similar to what I was thinking to fix this specific use case for the block library.: process?.env?.IS_GUTENBERG_PLUGIN ?? false.

My understanding is that we cannot use import.meta unless we ship our scripts as modules. Both vite and webpack favor this approach these days, but people find it problematic with many libraries and end up defining the process.env themselves — like we do in Gutenberg and people recommend for vite.

This is the options I see:

1. Document in the framework docs (this PR).
2. Make it not to break just for the @wordpress/block-library package: process?.env?.IS_GUTENBERG_PLUGIN ?? false. I don't find this approach sustainable in the whole codebase, though.
3. Update the variable to a IS_GUTENBERG_PLUGIN global.

I'd like to summon @gziolo @jsnajdr @swissspidy @talldan @t-hamano that have participated in similar conversations, in case they have thoughts/advice.

My understanding is that we cannot use import.meta unless we ship our scripts as modules.

FWIW, I think we already publish our scripts as modules. So is the idea to use process.env for the CJS build and import.meta for the modules build? That might be the best approach no?

IDK why Gutenberg uses process.env.IS_GUTENBERG_PLUGIN, but just using a IS_GUTENBERG_PLUGIN global (like the SCRIPT_DEBUG one that was added recently) makes more sense. Web scripts shouldn't artificially introduce a process.env that doesn't exist. Also weird that the global isn't replaced in build scripts 🤔

like we do in Gutenberg and vitejs/vite#1973.

Tip: when linking to files on GitHub, copy its permalink via the three-dots menu or press Y on the keyboard to get the permalink. This way we can see the file in the version at the time of writing, even when the file is later modified or moved.

Web scripts shouldn't artificially introduce a process.env that doesn't exist

For bundled scripts yes, but packages can ship and support environment variables. process.env variables have always been the standard in the JS community for env variables but lately import.meta is becoming the standard when it comes to ES modules.

Personally, I'd love if we can experiment with this #60087 (comment) and use consistently for all of our env variables. SCRIPT_DEBUG is a runtime variable, so it's different.

That said, I don't want to block this PR more. I'm fine with mentioning the variable in our docs temporarily while we figure this out.

I mentioned it in slack, but will mention it here too. There's also usage of process.env.NODE_ENV in the codebase (in packages like blocks, components, data).

I don't know if build tools handle that property in a special way or if it also causes the same issues as IS_GUTENBERG_PLUGIN. It might be the latter, but it's just that IS_GUTENBERG_PLUGIN causes an error first.

Just mentioning it as I'd hate to see effort go into fixing IS_GUTENBERG_PLUGIN but then later realize there's still a problem with NODE_ENV.

When using "Vanilla" starter index.html imports main.js and there is no index.jsx. Additionally, since Vite doesn’t automatically apply JSX transforms to js files we’d probably want to do something like:

It’s one more small reason to prefer the "React" starter as relevant files will already be .jsx. Then we can say to put the code in src/App.jsx and also remove the bits about createRoot and render because those will already exist in src/main.jsx.

I'd be happy to put the switch to "React" starter in a follow-up PR though because, well, like a goof I already created one that does that (and has many of the changes in this one) because I didn’t check first to see if it was being worked on.