search

xfiveco / generator-chisel

Chisel is a development framework for creating easy to maintain and fast WordPress websites.
https://www.getchisel.co
MIT License
269 stars 37 forks source link

Feature/dependencies update #500 #503

Open edenizk opened 3 years ago

edenizk commented 3 years ago

Updated packages + I made changes for the new version of webpack and webpack plugins.

jakub300 commented 3 years ago

I'll take a lok over the weekend

jakub300 commented 3 years ago

Hi, I haven't tested it but I noticed that tests are failing (https://www.travis-ci.com/github/edenizk/generator-chisel/builds), please take a look. Some could be fixed by merging #501. Some (like different hashes) are expected and need to be updated.

jakub300 commented 3 years ago

Hi, I had some time to test yesterday and today, few notes

General (both static and WP)

  1. It looks like you don't have prettier enabled in you editor, please enable and reformat edited files.
  2. css is not minified
  3. dynamic public path is disabled right now async scripts seem to be forking in wp thanks to webpacks automatic dynamic path, not sure if we should use our or webpacks. I prepared fixed version for webpack 5 earlier and some other improvements https://github.com/xfiveco/generator-chisel/compare/2afd660c0...6a371bb31a
  4. It's listening on localhost by default so accessing over local network is not possible and when using some kind of proxy wps still has hardcoded url (not tested) so would not work

Static

  1. serveDist is not working
  2. skipHtmlExtension is not working
  3. errors when tried async imports import('./modules/greeting')
  4. ~I briefly tried react in static and got React is not defined errors~ disregard that, we don't enable automatic runtime right now

WP

This is interesting browser sync and serve are listening on the same port but it seems to be working, not sure how. Do you have some understanding here, is that intentional or just happened by accident?

edenizk commented 3 years ago

Hi @jakub300, Thank you for reviewing my code, here is my update and some questions about some tasks:

General (both static and WP)

1- Thank you for letting me know I have enabled it and formatted it. 2- I've been looking today at the mini-css-extract-plugin, but couldn't find a problem. When you were checking have you seen any clue, why it might not be working? 3- I have added your solution, since webpack 5 is still changing maybe it's better if we have our solution. Thank you for preparing them 🙂. After adding your changes, watching stopped working tho. Does it work on your end? 4- For WP I see that it is working due to Browsersync. For the static, I have added a hook for setting the host with an external URL(I think I can simplify it tho since we are only using for static). The only issue unfortunately I couldn't find loopback support in the WPS, like in the WDS. If I will set the external URL, everything will work fine(HMR will work in external URL + in the local machine), but it won't be possible to view the page on localhost:3000. What would you suggest me to do?

Static

2- Solved 3-Solved 4-Would it be possible for you to send me the error message, and the code you are trying? 5- Is react problem continue? Because when I tested it was working fine on my end. If there is still a problem could you share with me your react setup? And what do you mean about the automatic runtime?

WP

It wasn't sth I have noticed from WPS documentation, but sth I have tried after noticing the WebSocket option from browser-sync. After turning off the WebSocket for the browser-sync they can be worked in the same port without interrupting each other 🙂

jakub300 commented 3 years ago

Great progress, I did a bit of testing, next round I'll look more into the code.

General (both static and WP)

2- I've been looking today at the mini-css-extract-plugin, but couldn't find a problem. When you were checking have you seen any clue, why it might not be working?

packages\chisel-scripts\lib\webpack-plugins\OptimizeCssnanoPlugin.js is responsible for minification. It's based on plugin linked at the top of the file with added filtering for .full files. Looks like it doesn't support Webpack 5, Internet suggests https://github.com/webpack-contrib/css-minimizer-webpack-plugin

3- I have added your solution, since webpack 5 is still changing maybe it's better if we have our solution. Thank you for preparing them 🙂. After adding your changes, watching stopped working tho. Does it work on your end?

Seems to be working, just tested on WP. Could you share more details?

4- For WP I see that it is working due to Browsersync. For the static, I have added a hook for setting the host with an external URL(I think I can simplify it tho since we are only using for static). The only issue unfortunately I couldn't find loopback support in the WPS, like in the WDS. If I will set the external URL, everything will work fine(HMR will work in external URL + in the local machine), but it won't be possible to view the page on localhost:3000. What would you suggest me to do?

I see, I guess it could be improved in WPS.

Static

4-Would it be possible for you to send me the error message, and the code you are trying?

In generated project replace app.js contents with:

import('./modules/greeting').then(({ default: greet }) => {
  greet('Worldddd');
});

WP

It wasn't sth I have noticed from WPS documentation, but sth I have tried after noticing the WebSocket option from browser-sync. After turning off the WebSocket for the browser-sync they can be worked in the same port without interrupting each other 🙂

This is weird, normally two different servers should not be able to listen on the same port at the same time. In CI I see (https://app.travis-ci.com/github/jakub300/generator-chisel/jobs/538291746#L879) it shows and error, are you able to test not on windows? because, this error makes more sense to me than what's happening on Windows.

edenizk commented 3 years ago

Hi @jakub300,

2- packages\chisel-scripts\lib\webpack-plugins\OptimizeCssnanoPlugin.js is responsible for minification. It's based on plugin linked at the top of the file with added filtering for .full files. Looks like it doesn't support Webpack 5, Internet suggests https://github.com/webpack-contrib/css-minimizer-webpack-plugin

I have replaced the cssnano with the CSS minimizer, everything looks fine now.

3- Seems to be working, just tested on WP. Could you share more details?

I have tested in FE only, when I've changed styling it didn't hot reload

4- I see, I guess it could be improved in WPS.

So I am keeping it with my last changes :)

Static

4- In generated project replace app.js contents with: import('./modules/greeting').then(({ default: greet }) => { greet('Worldddd'); });

It is due to the InjectRevisioned hook, I think because of the changes in a compilation. I have replaced it with the assets path from processAssets hook. But I want to be sure, is my changes what we want from this hook to do? Because this hook was a little bit unclear to me. It is generating sth like this { 'styles/main.css': 'styles/main.css' }

WP

This is weird, normally two different servers should not be able to listen on the same port at the same time. In CI I see (https://app.travis-ci.com/github/jakub300/generator-chisel/jobs/538291746#L879) it shows and error, are you able to test not on windows? because, this error makes more sense to me than what's happening on Windows.

After some more deep search, I think this flag https://lwn.net/Articles/542629/ is missing in the test machine. Unfortunately, currently I can only test on the Windows.