Skip to content

Conversation

kmalakoff
Copy link
Contributor

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 added 10 commits June 1, 2021 02:03
- use react-refresh plugin and loader
- user react-refresh only on client config, not dev wrapper builds
- update dynamic cdn plugin to exclude react externalization for development
- update devServer config to use "hot" mode and remove google-apps-script-webpack-dev-server "before" settings
also update webpack to latest version 4
@enuchi
Copy link
Owner

enuchi commented Jul 4, 2021

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
Copy link
Owner

enuchi commented Jul 4, 2021

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
Copy link
Contributor Author

kmalakoff commented Jul 4, 2021

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
Copy link

kdar commented Jul 4, 2021

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

@kmalakoff
Copy link
Contributor Author

@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
Copy link
Contributor Author

@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
Copy link
Owner

enuchi commented Jul 12, 2021

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
Copy link
Contributor Author

@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
Copy link
Owner

enuchi commented Jul 14, 2021

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 icelam/html-inline-script-webpack-plugin#66? Or any idea why it would not work in development with this plugin?

See 8fd64ce. Can you try this change locally and see if hot refresh works for you?

@enuchi
Copy link
Owner

enuchi commented Jul 14, 2021

Or like this: 5b18bed

@kmalakoff
Copy link
Contributor Author

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 kmalakoff@dd7c1d4

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

@kmalakoff
Copy link
Contributor Author

@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
Copy link
Owner

enuchi commented Jul 24, 2021

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

  1. Add integration tests to make sure HMR works across different operating systems when merging new changes
  2. Merge in improved react-refresh and typescript improvements
  3. 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
Copy link
Contributor Author

kmalakoff commented Jul 24, 2021

Thank you for explaining your plan. It sounds solid.

@kmalakoff
Copy link
Contributor Author

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
Copy link

darakeon commented Mar 7, 2022

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

Screen Shot 2022-03-07 at 11 41 52

@enuchi enuchi mentioned this pull request Apr 4, 2022
@enuchi
Copy link
Owner

enuchi commented Apr 4, 2022

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
Copy link
Owner

enuchi commented Jun 20, 2022

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
Copy link
Owner

enuchi commented Jun 20, 2022

@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
Copy link
Owner

enuchi commented Jun 20, 2022

Closing -- #131 has been merged.

@enuchi enuchi closed this Jun 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants