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

How to alias React and ReactDOM as wp.element? #13

Closed
cliffordp opened this issue Mar 19, 2020 · 12 comments
Closed

How to alias React and ReactDOM as wp.element? #13

cliffordp opened this issue Mar 19, 2020 · 12 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed question Further information is requested

Comments

@cliffordp
Copy link
Contributor

Continuing from parcel-bundler/parcel#144 (comment)

Maybe I'm not doing it just right or not understanding how things work, but here's what I've got so far:

The issue here is that WordPress is already loading React and ReactDOM via the @wordpress/element package from yoursite-com/wp-includes/js/dist/element.js (WordPress core, not my own path, as you can see).

Here's my package.json:

{
	...
	"dependencies": {
		"react-notifications-component": "^2.3.0"
	},
	"devDependencies": {
		"@babel/core": "^7.8.7",
		"@wordpress/scripts": "^7.1.2",
		"npm-run-all": "^4.1.5",
		"parcel-bundler": "^1.12.4",
		"parcel-plugin-externals": "^0.3.3",
		"postcss-modules": "^1.5.0"
	},
	"babel": {
		"presets": [
			"@wordpress/default"
		]
	},
	"browserslist": [
		"extends @wordpress/browserslist-config"
	],
	"externals": "./.externals.js"
}

Here's the .externals.js that my package.json runs:

const rxWp = /node_modules\/@wordpress\/(.*?)\//;
const rxBabel = /node_modules\/@babel\/(.*?)\//;
const rxReact = /node_modules\/react-(.*?)\//;

module.exports = function( path ) {
	const wp = rxWp.exec( path );
	const babel = rxBabel.exec( path );
	const react = rxReact.exec( path );

	let wpSuffix;

	let babelSuffix;

	let reactSuffix;
	let reactName;

	if ( wp ) {
		wpSuffix = wp[ 1 ];
		return `@wordpress/${wpSuffix} => require('@wordpress/${wpSuffix}')`;
	}

	if ( react ) {
		reactSuffix = react[ 1 ];
		reactName = reactSuffix[ 0 ].toUpperCase() + reactSuffix.substr( 1 );
		return `react-${reactSuffix} => React${reactName}`;
	}

	if ( babel ) {
		babelSuffix = babel[ 1 ];
		return `@babel/${babelSuffix} => require('@babel/${babelSuffix}')`;
	}

	return undefined;
};
@FlorianRappl
Copy link
Owner

Hm can the rxReact catch React at all? It seems to be able to catch things like react-dom, but not react (it requires another part after a dash).

The other thing is that you may want to exclude @wordpress/element. If I understand this one brings React and React-DOM as globals already (but maybe I misunderstand).

Would it be possible to make a MWE of your solution? I'd like to better understand the current setup to give you a better advice. Maybe we can also enhance the API of the plugin to cover this case.

Hope that helps!

@FlorianRappl FlorianRappl added documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed question Further information is requested labels Mar 19, 2020
@cliffordp
Copy link
Contributor Author

OK, I've made progress! :)

Reference: https://parceljs.org/module_resolution.html#aliases

"alias": {
	"react": "@wordpress/element",
	"react-dom": "@wordpress/element"
},
"externals": {
	"@wordpress/element": "wp.element"
}

Now the issue is my 3rd party component isn't satisfied :( https://www.npmjs.com/package/react-notifications-component

Any ideas why not???

image

FYI: your repo's (and this discussion's initial) code snippet includes "node_modules" in the regex match rule but path never starts with this. console.log( path ); worked a treat to see this.

@FlorianRappl
Copy link
Owner

FlorianRappl commented Mar 19, 2020

your repo's (and this discussion's initial) code snippet includes "node_modules" in the regex match rule

That's a great catch. Do you want to contribute a PR for this? Would be awesome to improve the docs. Otherwise, I'll correct this.

Any ideas why not???

Hm since we take the React UMD I could assume it does not come with a default export. Not sure though. To see if this is indeed the case you can try two things:

  1. "Hack" the react-notifications-component in your node_modules to use import * as React from 'react' instead of import React from 'react'
  2. Place in some code before react-notifications-component is imported / running the following snippet: require('react').default = require('react');

If either one or the other is suddenly working this is indeed the case. The second approach could be a way to circumvent this.

@cliffordp
Copy link
Contributor Author

I'll do PR after I get things straight here in case there are any other tips to add to the readme.

Man, nothing I tried worked... I'm not sure how I can edit what comes down from npm install

The github repo does not come down raw but instead already built by webpack so I don't know how it could be edited. And the 2nd idea you had didn't fix it either.

image

@cliffordp
Copy link
Contributor Author

I'm facing a deadline and really appreciate your quick help :D

Do you think https://parceljs.org/cli.html#expose-modules-as-umd is relevant at all? A quick try didn't work quite as well as yours but maybe I'm not using it correctly for my use case.

@FlorianRappl
Copy link
Owner

Ok that it was prebuilt using Webpack may explain this. I would need to study the code, but unfortunately I am fully covered with meetings today - so I'll either catch a look later tonight or tomorrow ... sorry!

You can have a look at the code from the package how React is resolved there. Is it coming from the global? What name should be used there? Check the dependencies of the package.json of the external package, too - React should not be bundled in and marked as a peer dependency.

@cliffordp
Copy link
Contributor Author

Tyvm. Yes it has react as a peer dependency:

https://github.com/teodosii/react-notifications-component/blob/master/package.json

@cliffordp
Copy link
Contributor Author

cliffordp commented Mar 20, 2020

Removing the "alias" from package.json seems to be the best out-of-the-box quick fix (i.e. no custom Externals JS and no modifying the npm dependency).

6min trial and error video, if you care to review: https://share.getcloudapp.com/5zuyOqkD

@cliffordp
Copy link
Contributor Author

Great news! I got it sorted out. It was as simple as this:

"externals": {
	"@wordpress/element": "wp.element"
}

No "alias" or anything else!

I think the issues I was experiencing that caused me to think something wasn't working correctly was that my API validation was incorrect so stuff wasn't working because of that.

THANK YOU!

@cliffordp
Copy link
Contributor Author

I submitted #14 -- thanks much for this package and your assistance!

@FlorianRappl
Copy link
Owner

Hi sorry for my late appearance. Wonderful that you made it work 🚀!

Also thanks for your PR - much appreciated 🍻!

@cliffordp
Copy link
Contributor Author

With your help:

https://github.com/cliffordp/cliff-wp-plugin-boilerplate#march-22-2020

Improve the JavaScript build for WordPress React, reducing the admin-settings.js file size:

  • Minified: from 265.91 KB to 35.16 KB (87% reduction)
  • Unminified: from 803.68 KB to 48.15 KB (94% reduction)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants