Created init.js script

[igmoweb]

Issue: #12

The new scripts/init.js script...

• Replace the default npm start script for the custom react-wp-scripts start
• Copy loader.php to the project root folder.

To make things easier, this PR can be tested by running:
create-react-app --scripts-version git+https://git@github.com/humanmade/react-wp-scripts.git#12-scripts-version-compatibility .

If you want to play more without installing it over and over, create a JS, for instance init-debug.js in the project root folder with the following content:

const init = require( 'react-wp-scripts/scripts/init.js' );
init( PATH_TO_PROJECT_ROOT_FOLDER );

And execute node ./init-debug.js. This will execute scripts/init.js.

[rmccue]

Does this not also have to call react-scripts's init.js script somewhere? 🤔

[igmoweb]

@rmccue Whoops, missed that. I just fixed it.

[igmoweb]

@kadamwhite Is this ok to merge?

[tfrommen]

Hm, I don't really like that I (as a user of this) don't have any option to decide where the loader.php file should go. I wouldn't want it to live in my plugin root.

Also, loader is a strange name, and sounds like a class. However, the file includes functions. Why not call it react-wp-scripts.php, or maybe even react-wp-scripts-functions.php? This is to the point, matches the name(space), and doesn't require any renaming either.

Lastly, the readme section about how to integrate this into a plugin has been merged with the theme one. This doesn't work, though, as you'd need to pass different paths (i.e., get_stylesheet_directory() vs. plugin_dir_path( __FILE__ )). Was this a mistake?

[rmccue]

@tfrommen This is just a project generation command, so you can always rename it after you've set up the project.

Lastly, the readme section about how to integrate this into a plugin has been merged with the theme one.

Looks like we could use the relevant plugin code; I'd add it into the same example but commented out.

[tfrommen]

Also see the related discussion in #21.

[igmoweb]

I've pushed some changes:

• Specify a --php-namespace argument. This will replace the %%NAMESPACE%% string in loader.php for the specified namespace. Default is still ReactWPScripts. There is not an error management if the replacement fails, I'll address that later if you see the changes fine.

• I've updated the README but I'm not completely sure if this was what you were looking for.

• The copied file has been now renamed to react-wp-scripts, a bit more meaningful.

@tfrommen as we have spoken already, I'm not for moving the copied file to another location. This is thought for starter projects and, though maybe moving the file to the root folder is not the best idea, the developer hasn't got yet a folder structure when this is installed so I think is better if we let the dev to do whatever it liked with that file.

Before moving on to add better error handling, any thoughts?

[rmccue]

@igmoweb Is there any reason not to autogenerate the namespace as well? We could also add a very minimal scaffold of the functions.php that just loads that file and hooks it in.

[rmccue]

I need this merged for the announcement post. :)

[rmccue]

Indentation is off here.

[rmccue]

No need for a ; here

[igmoweb]

@rmccue Ready. I got the package name and upper camel cased it by using a new package. I used a try-catch approach in case the package.json is not properly parsed for some reason and fallback in ReactWPScripts namespace by default.

[kadamwhite]

I think the error handling here is a bit inconsistent. If we consider a read error here to be a fatal error, we should throw that error so that it will be picked up by the catch below, or at minimum return after we log it to the console.

We also don't return a promise from this first .then, so the success message will likely display before this action has necessarily completed.

This is untested but I think we'd be looking for something more like this:

fs.copy( loaderPath, destinationFile )
  .then( () => new Promise(( resolve, reject ) => {
    // Replace %%NAMESPACE%% for the specified namespace
    fs.readFile( destinationFile, 'utf8', function( err, data ) {
      if ( err ) {
        return reject( err );
      }

      var result = data.replace( '%%NAMESPACE%%', namespace );
      fs.writeFile( destinationFile, result, 'utf8', function( err ) {
        if ( err ) {
          return reject( err );
        }
        resolve();
      } );
    } );
  } ) )
  .then( () => successMessage() )
  .catch( err => {
    console.log( chalk.bgRed( 'React WP Scripts loader could not be copied to your root folder. Error details:' ) );
    console.log( chalk.red( err ) );
  } );

[kadamwhite]

Actually I didn't realize initially but these docs have lagged, too -- the --php-namespace argument's been removed entirely in favor of parsing the project name. I kind of liked making that configurable, but at minimum we should explain that the namespace does NOT default to ReactWPScripts unless the packageJSON cannot be parsed, in which case the init is likely to fail anyway.

I honestly don't know that I like auto-generating the namespace from the project; @rmccue why did you feel it was preferable to using an opinionated default like ReactWPScripts? If I have a project H2, I wouldn't want the namespace for my script-loader file to be H2.

[tfrommen]

@kadamwhite see discussion on #21. I initially introduced a simple guard, because I see it pretty much like you. The functions are part of the react-wp-script package, and not of my app, whatever it is that I am using react-wp-scripts with.

@rmccue then mentioned the possibility to allow for multiple (potentially different) version of the functions being used. Not sure if this is to be aimed for at all, but that was where our ideas/approaches diverged.

[rmccue]

If I have a project H2, I wouldn't want the namespace for my script-loader file to be H2.

On the contrary, that's exactly what I want. After init, the loader file is part of my code, not a library being loaded in. It should match that.

[kadamwhite]

We could also add a very minimal scaffold of the functions.php that just loads that file and hooks it in.

This feels like overkill. I'm inclined to limit the scope of react-wp-scripts to "bridge the WP space with the Webpack space," rather than trying to make a one-size-fits-all scaffolding tool; Yeoman and grunt-init had their fans and have their uses, but I believe it's a better use of our time to make a separate plugin or theme boilerplate project than to try to make this module do too much

[kadamwhite]

Wrapped up my feedback into #25, which is PR'd against this branch

[rmccue]

This feels like overkill. I'm inclined to limit the scope of react-wp-scripts to "bridge the WP space with the Webpack space," rather than trying to make a one-size-fits-all scaffolding tool; Yeoman and grunt-init had their fans and have their uses, but I believe it's a better use of our time to make a separate plugin or theme boilerplate project than to try to make this module do too much

I disagree with that scope fundamentally. If all we wanted to do was bridge the gap of loading Webpack assets in WordPress, we could have written a blog post about asset-manifest.json and been done. But we're specifically trying to tackle the problem that create-react-app tackled: ease of getting a new project started.

When I type create-react-wp-plugin . I expect to have a working plugin, not half a plugin. To that end, we should create a plugin.php for that. If you don't like it, you can just delete it after generation, the same way you can delete create-react-app's tests or registerServiceWorker if you don't want them.

Where this differs from Yeoman &c is that it's not for full scaffolding, it's just a bare minimum setup to get something running. You can still use those if you want to scaffold out a full project if you really want, but this is meant to be only a starter.

[kadamwhite]

This is not create-react-wp-plugin, though, just as react-scripts is not create-react-app. I’m 100% for the existence of this tool but I think there’s a separation between the CLI and the helper utility, and I would like for this to exist independently even if it ends up being but a small part of a larger, more complex repository.

[rmccue]

That's true, but init.js exists only for create-react-app, so I think it's relevant. If you manually add react-wp-scripts to your existing app, then none of this PR matters.

I've already started on writing those commands using a Lerna setup similar to CRA's, so react-wp-scripts would live in /packages/react-wp-scripts. I think we should always allow adding react-wp-scripts to existing projects, but I also think it makes sense to home all of the related pieces together.

[kadamwhite]

In that case I’d like to see #25 and #15 merged in their current form (allowing a manual override and defaulting to a set string), then to introduce the option to automatically set the namespace or prompt the user during script initialization once that script exists. At this time we’re still working on a wrapper for react-scripts, and I think everything here fits pretty well into that even if we plan to expand it in the very near term.

[kadamwhite]

• once that CLI exists

[rmccue]

In that case I’d like to see #25 and #15 merged in their current form (allowing a manual override and defaulting to a set string)

I guess I just don't see what the point of that is; init.js is only needed for either create-react-app --scripts-version or create-react-wp-*, and once the latter exists, we have no need to even mention the former.

To think from another angle here: what's the benefit of using a static string for the namespace? By doing so we encourage a bad practice which leads to duplicate-declaration conflicts.

[kadamwhite]

/shrug

I like simplicity and configurability (whether a CLI argument or a user prompt) a lot more than I like magic, but I turn #25 and this PR over to you, do with them as you will.

[rmccue]

As I mentioned in Slack:

I think the key to it is that the configuration doesn't matter much, since you can just edit the file. I'll merge all your PRs for now, since they don't really affect what I'm working on, but we should continue discussing 🙂
I think it might be easier to discuss once the concrete create-react-wp-plugin/theme commands exist