Issue/webpack 5 upgrade #106
Conversation
There was a problem hiding this comment.
Really great work here so far. With limited issues I was able to get one of your widget PRs running locally and installed to dev Materia.
Some high-level comments:
- As commented, MWD has been updated to hopefully (?) include the most recent/missing dependencies. It's worth confirming 0.2.0 works out of the box, and replace the reference to it in the package.json when possible.
- I love the little progress bar when installing to local Materia. That's a nice touch.
- I haven't tested the score screen feature, but the instructions could be a little clearer as to what's required. If the score screen options aren't ready for prime-time, we could also potentially disable them until they have more time in the oven.
- Previously the MWDK would allow you to save draft/publish a qset to test in the player. Right now it looks like the demo is the only qset that works with the player. Is that the case? Are there significant challenges to re-implementing cached qsets?
- Would it be possible to render Player and Creator guides, if they exist, within the MWDK? It looks like we already have view templates to support them.
Otherwise, just a little bit of polish based on feedback items, and making sure all current changes on this branch are final, required, and complete, and it looks like it might be good to go.
express.js
Outdated
| // 2. filter for materia-web image and named xxxx_phpfpm_1 name | ||
| // 3. pick the first line | ||
| // 4. pick the container name | ||
| let targetImage = execSync('docker ps -a --format "{{.Image}} {{.Names}}" | grep -e ".*materia:.* docker_app_.*" | head -n 1 | cut -d" " -f2'); |
There was a problem hiding this comment.
This line continues to give us problems. Some versions of docker-compose use hyphens, others use underscores, and the server does not handle a search failure well. We can modify the search pattern grep is using to: ".*materia:.* docker[-_]app[-_].*" which will catch either option.
There was a problem hiding this comment.
Also it'd be nice if the server gave a more well-handled response if the docker install process runs into an issue
|
Thanks for the feedback! New changes:
|
There was a problem hiding this comment.
Super minor nitpick with some inline spacing, but nothing I'd hold up a release for.
I couldn't get the score screen working with valid JSON, but considering the latest version of the MWDK doesn't have a functional score screen either I'm not sure I'd hold up a release for that, either.
Everything looks like it's working well otherwise.
express.js
Outdated
| const path = require('path'); | ||
| const fs = require('fs'); | ||
| const express = require('express') | ||
| const qsets = path.join(__dirname, 'qsets'); | ||
| const yaml = require('yamljs'); | ||
| const { execSync } = require('child_process'); | ||
| const waitUntil = require('wait-until-promise').default | ||
| const { v4: uuidv4 } = require('uuid') | ||
| const sharp = require('sharp') | ||
| const util = require('util'); | ||
| const cors = require('cors') | ||
| const hbs = require('hbs'); | ||
|
|
||
| // common paths used here | ||
| const srcPath = path.join(process.cwd(), 'src') + path.sep | ||
| const outputPath = path.join(process.cwd(), 'build') + path.sep | ||
|
|
||
| // Webpack middleware setup | ||
| const webpack = require('webpack'); | ||
| const webpackDevMiddleware = require('webpack-dev-middleware'); |
There was a problem hiding this comment.
This doesn't really matter, but stick to spaces for inline alignment.
Would prefer to remove all the extraneous white space, personally.
MWDK Upgrade
webpack-dev-middlewareto compile and view assets.Requirements:
Local setup
### Materia The current Materia widget-dependencies-package does not support the MWDK. UPDATE: the MWD package has been updated. **These steps are no longer needed**. 1. Checkout this [react/widget-dependencies-package-part-three branch](https://github.com/cayb0rg/Materia/tree/react/widget-dependencies-package-part-three) of Materia. This is required to load the player and creator. 2. Install Materia if you have not. 3. Run ``yarn build`` inside Materia's root folder. 4. Run ``yarn link`` inside Materia/public/dist. 5. Start Materia (run ``docker-compose up`` in Materia/docker and ``yarn dev`` in Materia/src)Widget
Each widget may need a few small changes to support Webpack v5. Here is an incomprehensive list of the possible changes:
It should look similar to:
moduleRules, if applicable:rules.loadAndCompileMarkdownandrules.loadAndPrefixCSSpackage.jsonto:Look at an example for the changes that need to be made (ignore the images).
AngularJS Widgets
AngularJS widgets may encounter an angular injection error. This usually only occurs with widgets that have directives, services, and controllers loaded in separate files. There are two ways to fix this. One way is to do what This or That does, and set up the directives, controllers, and services in a single file (for example, see creator.js. The other, more difficult way I've found is to manually bootstrap the angular modules.
ng-appfrom body or HTML tags.player.jsandcreator.jsto the end of the body tagangular.bootstrap(document, ['your-module-name-here']). The example above would look like:Look at an example for the changes that need to be made.
Running this MWDK PR
yarn install && yarn build && yarn linkinside the Materia Widget Development Kit. You may also usebun install.rm -rf node_modulesinside your widget."materia-widget-development-kit": "https://github.com/cayb0rg/Materia-Widget-Dev-Kit.git#issue/webpack-5-upgrade"yarn installorbun installyarn link materia-widget-development-kit. This will allow you to make changes to the MWDK without having to reinstall it in your widget.yarn startorbun startKnown Bugs / To-Do
Angular widgets do not work.(see above)Not updating the title of a widget may cause an error when trying to preview it.Question importer does not work.(fixed)MWDK does not run independently of widgets.(fixed)Media importer creates incorrect urls in creator(client issue)Issues resolving babel-loader for React widgets(client issue)Getting a Compilation.assets deprecation error in console(fixed)Updatewebpack-generate-widget-hashto use compiler.hooks.compilation.tapScreenshot annotation tool(fixed)Icomoon fonts get a 404 once installed in MateriaUpdated Widgets (PRs made)