Webpack 5 support - Githubissues

kmalakoff commented 3 years ago

I've been using your boilerplate and tried upgrading the dependencies to the latest version. Thank you for your amazing work!

I got it to work with webpack 5...almost. Hot module reloading isn't working, but after trying all the fixes and workarounds I could find and it still not working, I'm wondering if you want to try to get it to work? It's so close!

To upgrade to webpack 5, I needed to rewrite dynamic-cdn-webpack-plugin and published it under a new module @effortlessmotion/dynamic-cdn-webpack-plugin until the author accepts the pull request.

enuchi commented 3 years ago

Thanks @kmalakoff, appreciate this. I've avoided updating to webpack 5 because of the dynamic-cdn-webpack-plugin issue and happy to see you've published a fork. That repo hasn't had activity in years so a bit doubtful it will get merged but curious to see.

I've made some changes to the way hot module reloading works, and it should be more straightforward/modern, just haven't merged in yet. Can you try applying the changes in your PR to my branch at fix/update-webpack-dev-server (see PR #98) and seeing if hot refresh works with the new setup and webpack 5?

enuchi commented 3 years ago

I believe I also had issues with html-webpack-inline-source-plugin when trying to upgrade webpack. Is that why you switched to the other inline plugin? It didn't look like it had as much support...

kmalakoff commented 3 years ago

Good question! Choosing html-inline-script-webpack-plugin was mainly a "process of elimination" to find a plugin compatible with webpack 5. If there's a more common one out there with webpack 5 support, we can swap it in.

html-webpack-inline-source-plugin doesn't support webpack 5 and hasn't been updated for 3 years:

This plugin is no longer maintained. Facebook provides a similar plugin: https://github.com/facebook/create-react-app/blob/edc671eeea6b7d26ac3f1eb2050e50f75cf9ad5d/packages/react-dev-utils/InlineChunkHtmlPlugin.js#L10

Unfortunately, InlineChunkHtmlPlugin hasn't been updated for 2 years and so also doesn't support webpack 5.

kdar commented 3 years ago

You can use https://github.com/KyLeoHC/inline-source-webpack-plugin with webpackv5.

kmalakoff commented 3 years ago

@kdar thank you for the tip.

Based on npm downloads, it looks like html-inline-script-webpack-plugin is more popular than inline-source-webpack-plugin.

Also, I tried out inline-source-webpack-plugin and it is expecting a non-standard signature to mark the replacement rather than a standard src attribute:

<script inline inline-asset="runtime\.\w+\.js$" inline-asset-delete></script>

I wasn't able to get it to work quickly either ;-P

kmalakoff commented 3 years ago

@enuchi I just realized that you requested me to try updating fix/update-webpack-dev-server (PR #98)

I tried it and I ran into the same issue with hot-update endpoints missing. See commit

Also, something weird happened that I tried to chase down, but couldn't figure it out. In the generated code.js file, there was incorrectly generated code:

    (() => {
        var exports = __webpack_exports__;
        exports.__esModule = !0, exports.setActiveSheet = exports.deleteSheet = exports.addSheet = exports.getSheetsData = exports.openAboutSidebar = exports.openDialogBootstrap = exports.openDialog = exports.onOpen = void 0;
        var ui_1 = __webpack_require__(1);
        exports.onOpen = ui_1.onOpen, exports.openDialog = ui_1.openDialog, exports.openDialogBootstrap = ui_1.openDialogBootstrap, 
        exports.openAboutSidebar = ui_1.openAboutSidebar;
        var sheets_1 = __webpack_require__(2);
        exports.getSheetsData = sheets_1.getSheetsData, exports.addSheet = sheets_1.addSheet, 
        exports.deleteSheet = sheets_1.deleteSheet, exports.setActiveSheet = sheets_1.sglobal.__esModule = exports.__esModule, 
        global.setActiveSheet = exports.setActiveSheet, global.onOpen = exports.onOpen, 
        global.openDialog = exports.openDialog, global.openDialogBootstrap = exports.openDialogBootstrap, 
        global.openAboutSidebar = exports.openAboutSidebar, global.getSheetsData = exports.getSheetsData, 
        global.addSheet = exports.addSheet, global.deleteSheet = exports.deleteSheet, etActiveSheet;
    })();

This had two invalid things: sheets_1.sglobal.__esModule and , etActiveSheet;. I'm not sure what caused the errors

enuchi commented 3 years ago

Hey @kmalakoff, think the server-side error is coming from changes to gas-webpack-plugin. I know @fossamagna made changes to that repo to support removing the "global" declarations but it doesn't seem to be working here. If I make the changes in this commit then the server-side works.

As for the hot refresh, it isn't even working outside of the Google spreadsheet. If you try running npm start and visit https://localhost:3000/dialog-demo-bootstrap-impl.html in the browser then you should be able to see hot refresh working. I don't know enough about the changes in webpack 5 and all the plugins you upgraded/swapped but something there is causing issues. Have you tried comparing the generated html files for clues?

kmalakoff commented 3 years ago

@enuchi I've spent quite a few hours mainly looking on the server side as the {chunk}.hot-update.js route seems to be missing (404) rather than looking at the generated html files. Is there something that you are seeing that makes the broken hot reloading seem to be client side?

enuchi commented 3 years ago

Okay @kmalakoff got this to work locally. Looks like there is some issue with html-inline-script-webpack-plugin that prevents hot refresh from working in development. Have not really investigated too much. Maybe https://github.com/icelam/html-inline-script-webpack-plugin/issues/66? Or any idea why it would not work in development with this plugin?

See https://github.com/enuchi/React-Google-Apps-Script/commit/8fd64ce9e91a0361eac66d0d8b5c1bbfb54e4e0c. Can you try this change locally and see if hot refresh works for you?

enuchi commented 3 years ago

Or like this: https://github.com/enuchi/React-Google-Apps-Script/commit/5b18bed63e19cf2b06eca7beb54bf5890f35c85a

kmalakoff commented 3 years ago

Hey @enuchi. I wasn't able to get your fixes working so I went back to the drawing board and ported html-webpack-inline-source-plugin to webpack 5 @effortlessmotion/html-webpack-inline-source-plugin.

Here's a working version based on your fix/update-webpack-dev-server branch https://github.com/kmalakoff/React-Google-Apps-Script/commit/dd7c1d41c8bdb31c1be77f24fadd69a879536355

I think it is should be super simple to merge in....just the webpack config and updated / new modules

kmalakoff commented 3 years ago

@enuchi I merged my changes into this branch based on your fix/update-webpack-dev-server branch. Hot reloading with webpack 5 works and I updated all the modules to the latest

Let me know if you prefer a different approach to any of this

enuchi commented 3 years ago

Hey @kmalakoff, thanks again for this work on upgrading to webpack 5. Have not forgotten about this. Would like to go in this order:

Add integration tests to make sure HMR works across different operating systems when merging new changes
Merge in improved react-refresh and typescript improvements
Merge in your work on upgrading to webpack 5 and updating packages

Currently working on the first one. Hang tight, and will comment here hopefully soon when I have updates! Thanks again.

kmalakoff commented 3 years ago

Thank you for explaining your plan. It sounds solid.

kmalakoff commented 3 years ago

Here’s how I added cross-platform testing using a free travisci account to one of the webpack plug-ins: https://github.com/kmalakoff/html-webpack-inline-source-plugin/blob/master/.travis.yml

darakeon commented 2 years ago

There is a vulnerability found because of a dependency of this library.

enuchi commented 2 years ago

Hi @kmalakoff, sorry it's taken a while to get back to this one. I finally got around to doing step 1 and 2. React-refresh and integration tests are now there. Any chance you are able to pull in the new changes? Also totally understand if you've moved on from this.

I am also happy to take a look and try to pull in all the change but may need to pull your work onto a new branch/PR so I can edit.

enuchi commented 2 years ago

Hi @kmalakoff, I've pulled in your changes here into a new PR, #131, with some additional minor changes. I know you did a lot of work on this last year and not sure if you're still following along, but anyway, thanks for adding webpack 5 support for dynamic-cdn-webpack-plugin and html-webpack-inline-source-plugin.

I've added integration tests to the repo with Github Actions that test the local development setup, and running them against a few versions of node and both mac/windows, so now have more confidence that the switch to webpack 5 works.

Will merge the other PR in a bit and close this one.

enuchi commented 2 years ago

@enuchi I just realized that you requested me to try updating fix/update-webpack-dev-server (PR #98)

I tried it and I ran into the same issue with hot-update endpoints missing. See commit

Also, something weird happened that I tried to chase down, but couldn't figure it out. In the generated code.js file, there was incorrectly generated code:

    (() => {
        var exports = __webpack_exports__;
        exports.__esModule = !0, exports.setActiveSheet = exports.deleteSheet = exports.addSheet = exports.getSheetsData = exports.openAboutSidebar = exports.openDialogBootstrap = exports.openDialog = exports.onOpen = void 0;
        var ui_1 = __webpack_require__(1);
        exports.onOpen = ui_1.onOpen, exports.openDialog = ui_1.openDialog, exports.openDialogBootstrap = ui_1.openDialogBootstrap, 
        exports.openAboutSidebar = ui_1.openAboutSidebar;
        var sheets_1 = __webpack_require__(2);
        exports.getSheetsData = sheets_1.getSheetsData, exports.addSheet = sheets_1.addSheet, 
        exports.deleteSheet = sheets_1.deleteSheet, exports.setActiveSheet = sheets_1.sglobal.__esModule = exports.__esModule, 
        global.setActiveSheet = exports.setActiveSheet, global.onOpen = exports.onOpen, 
        global.openDialog = exports.openDialog, global.openDialogBootstrap = exports.openDialogBootstrap, 
        global.openAboutSidebar = exports.openAboutSidebar, global.getSheetsData = exports.getSheetsData, 
        global.addSheet = exports.addSheet, global.deleteSheet = exports.deleteSheet, etActiveSheet;
    })();

This had two invalid things: sheets_1.sglobal.__esModule and , etActiveSheet;. I'm not sure what caused the errors

Looks like there were some issues with gas-webpack-plugin, and @fossamagna was able to publish a new version that fixed them (issue here).

enuchi commented 2 years ago

Closing -- #131 has been merged.

enuchi / React-Google-Apps-Script

Webpack 5 support #103