FlorianRappl / parcel-plugin-externals

Parcel plugin for declaring externals. These externals will not be bundled. :package:
https://piral.io
MIT License
47 stars 3 forks source link

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

Closed cliffordp closed 4 years ago

cliffordp commented 4 years ago

Continuing from https://github.com/parcel-bundler/parcel/issues/144#issuecomment-601017068

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 commented 4 years ago

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!

cliffordp commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

Tyvm. Yes it has react as a peer dependency:

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

cliffordp commented 4 years ago

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 commented 4 years ago

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 commented 4 years ago

I submitted https://github.com/FlorianRappl/parcel-plugin-externals/pull/14 -- thanks much for this package and your assistance!

FlorianRappl commented 4 years ago

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

Also thanks for your PR - much appreciated :beers:!

cliffordp commented 4 years ago

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)