Update Electron to latest: update webpack to vite#128
Update Electron to latest: update webpack to vite#128
Conversation
|
FYI for @strogonoff . This PR is still a draft and not yet ready. |
7757bfd to
de759ab
Compare
| config.optimization = { | ||
| ...config.optimization, | ||
| moduleIds: "named", | ||
| }; |
|
|
||
| config.optimization = { | ||
| ...config.optimization, | ||
| moduleIds: "named", |
There was a problem hiding this comment.
Expected '}' to match '{' from line 4 and instead saw ':'.
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
Unrecoverable syntax error. (29% scanned).
| config.resolve.modules = [path.resolve(__dirname, "./src"), "node_modules"]; | ||
|
|
||
| config.optimization = { | ||
| ...config.optimization, |
There was a problem hiding this comment.
Expected '}' to match '{' from line 7 and instead saw '...'.
Missing semicolon.
| const path = require('path'); | ||
| const ThreadsPlugin = require('threads-plugin'); | ||
| const path = require("path"); | ||
| const ThreadsPlugin = require("threads-plugin"); |
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
| @@ -1,28 +1,30 @@ | |||
| const path = require('path'); | |||
| const ThreadsPlugin = require('threads-plugin'); | |||
| const path = require("path"); | |||
There was a problem hiding this comment.
'const' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
|
@SmartWolf1220 Could you please list the steps needed to test & build the ITU OB Editor with your PRs, from clean slate, that you have tested? Including any pre-requisites, such as Node version(s). Note that I was able to run However, I am unable to run build the app itself (this repository). Node v12 that you mentioned before will not work, because some dependencies that you updated in this PR now require Node v18. However, after switching to Node v18, while cc @ronaldtse |
|
First, you can remove yarn.lock and node modules. |
|
|
If Yarn’s lockfile is outdated, please update this PR with the new lockfile that contains the required versions of the packages. I’ll pull the new lockfile and try with those versions. |
|
Wait for a moment. |
|
First, remove node_modules and yarn.lock. |
|
@SmartWolf1220 what’s going on? The build is still not ready? How come the build is still failing? |
|
@ronaldtse Wait a moment, let me test the latest changes on this PR since my last comment… If it builds, it builds. However, @SmartWolf1220 keep in mind that if the lockfile is outdated, it is expected that you will commit the up-to-date lockfile with exact package versions. Otherwise you’re leaving the exact versions of packages up to fate at build time, and that’s not great for reproducible builds. Clearing a lockfile means allowing all packages to update, which is both a security liability and can lead to broken builds. So, please, don’t ask us to delete the lockfile. If you want, you can delete and regenerate it yourself, in which case please commit the updated lockfile version. |
So far I am seeing this error after running |
|
@SmartWolf1220 Have you tested the steps you wrote? They may be missing the compile command… |
|
@SmartWolf1220 As of latest changes, after compiling the app with Coulomb version from your other PR, the .app can be launched, which is great. Removing lockfile was not necessary. @ronaldtse I believe I got the app to successfully launch on arm64. I will re-check and merge the PR. I guess next steps is we can see that it builds on GHA, including for Debian, and fix other blocking issues. |
|
For me to merge this PR it needs to not have draft status, though. |
|
I will need to go through all of the changes in this PR in order to make sure all is OK. There are a lot of stylistic changes (e.g., every single quote was changed to a double quote). Without them it would be much faster. I don’t know whether a bunch of changes were made because of that “hound” linter. If so, it must be turned off, because we don’t want to have to review 99% stylistic changes in every PR. cc @ronaldtse |
|
Apparently I can remove the draft status :) |
|
@strogonoff , I will check the layout. |
|
Anyway, is it working well on your side with node 18? |
|
How come the lint and tests jobs are both failing here? |
Add Linux Build
|
Hello @strogonoff , It works well on Linux Debian too. |
…into upgrade-vite
|
Hello @strogonoff , I think the test fails because of node version. |
@ronaldtse I did not update other jobs, just the build jobs. The other jobs are not that critical right now. Probably they use a wrong Node version. |
|
So, as of today, we have this:
Status of GHA build testing
cc @ronaldtse @SmartWolf1220 |
We can close them at any point. I don’t see them as particularly helpful. |
a0fe964 to
d0db796
Compare
| resolve: { | ||
| modules: [path.resolve(__dirname, "./src"), "node_modules"], | ||
| extensions: [".ts", ".tsx", ".js", ".jsx"], | ||
| alias: { |
There was a problem hiding this comment.
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
Unrecoverable syntax error. (35% scanned).
| ...config, | ||
| resolve: { | ||
| modules: [path.resolve(__dirname, "./src"), "node_modules"], | ||
| extensions: [".ts", ".tsx", ".js", ".jsx"], |
There was a problem hiding this comment.
Expected '}' to match '{' from line 7 and instead saw ':'.
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
| const newConfig = { | ||
| ...config, | ||
| resolve: { | ||
| modules: [path.resolve(__dirname, "./src"), "node_modules"], |
| // Create a fresh config object | ||
| const newConfig = { | ||
| ...config, | ||
| resolve: { |
There was a problem hiding this comment.
Expected '}' to match '{' from line 3 and instead saw ':'.
Expected an assignment or function call and instead saw an expression.
Missing semicolon.
| module.exports = function (config) { | ||
| // Create a fresh config object | ||
| const newConfig = { | ||
| ...config, |
There was a problem hiding this comment.
Expected '}' to match '{' from line 5 and instead saw '...'.
Missing semicolon.
| padding: 0 0.5rem; | ||
|
|
||
| .DayPicker-Day, .DayPicker-Day--disabled { | ||
| .DayPicker-Day, |
There was a problem hiding this comment.
Selector DayPicker-Day--disabled should be written in lowercase with hyphens
Selector DayPicker-Day should be written in lowercase with hyphens
Selector should have depth of applicability no greater than 2, but was 6
| $pubColorLight: #3dcc91; | ||
| $pubColorDark: #0f9960; | ||
| $cutColorLight: #ffb366; | ||
| $cutColorDark: #d9822b; |
There was a problem hiding this comment.
Name of variable cutColorDark should be written in all lowercase letters with hyphens instead of underscores
|
|
||
| $pubColorLight: #3dcc91; | ||
| $pubColorDark: #0f9960; | ||
| $cutColorLight: #ffb366; |
There was a problem hiding this comment.
Name of variable cutColorLight should be written in all lowercase letters with hyphens instead of underscores
| @import "./mixins"; | ||
|
|
||
| $pubColorLight: #3dcc91; | ||
| $pubColorDark: #0f9960; |
There was a problem hiding this comment.
Name of variable pubColorDark should be written in all lowercase letters with hyphens instead of underscores
| @use "~@blueprintjs/core/lib/scss/variables" as *; | ||
| @import "./mixins"; | ||
|
|
||
| $pubColorLight: #3dcc91; |
There was a problem hiding this comment.
Name of variable pubColorLight should be written in all lowercase letters with hyphens instead of underscores







update the electron to latest and migrate from webpack to vite.