Conversation
|
@leecalcote should i also upgrade mocha version ? please take a look |
| "scripts": { | ||
| "dev": "cross-env NODE_ENV=development webpack-dev-server -d --progress --colors", | ||
| "test": "cross-env NODE_ENV=test mocha --require ./test/babel-register.js --extensions js,jsx test/**/*.test.js", | ||
| "test": "cross-env NODE_ENV=test mocha --require ./test/swc-register.js --extension js --extension jsx test/**/*.test.js", |
There was a problem hiding this comment.
@saurabhraghuvanshii instead of the comma-separated values, i just added it twice, it matches mocha's current cli behavior
kishore08-07
left a comment
There was a problem hiding this comment.
@YASHMAHAKAL I noticed inconsistent module configurations across files: .swcrc and .swcrc.test are using commonjs, while rollup.config.js is configured for es6.
Could we align these configs or rely on Rollup to handle the module format to avoid potential build inconsistencies?
package.json
Outdated
| @@ -107,7 +97,6 @@ | |||
| "dependencies": { | |||
| "@babel/runtime-corejs3": "^7.12.1", | |||
There was a problem hiding this comment.
@YASHMAHAKAL @babel/runtime-corejs3 is still in dependencies. Can we remove this?
There was a problem hiding this comment.
@kishore08-07, i had a reason for this, but i thought on that reason again, and now i think i should be removing it, Thanks.
rollup.config.js
Outdated
| targets: { | ||
| ie: '11', | ||
| chrome: '58', | ||
| firefox: '54', | ||
| safari: '10', | ||
| edge: '15', |
There was a problem hiding this comment.
@YASHMAHAKAL shall we update to modern baselines?
There was a problem hiding this comment.
implemented, this made build even faster.. Thanks !
The different module types are intentional and follow the recommended pattern for Rollup + SWC. |
@YASHMAHAKAL Thanks for clarifying, that makes sense. |
|
@saurabhraghuvanshii keep working this through, please. |
4c3f9b3 to
4cc37ff
Compare
|
@YASHMAHAKAL, did you test your changes? Did you see any improvement in build time? |
|
@saurabhraghuvanshii yes, build time was reduced to 900 ms vs 2.5 seconds |
|
@saurabhraghuvanshii however, i just resolved merge conflicts so i'm testing it again |
Wow!! |
|
@saurabhraghuvanshii i have tested it again and new build time is - 1.2 seconds vs 4.5 seconds after having changes in master recently : ) |
.eslintrc
Outdated
| "settings": { | ||
| "react": { | ||
| "version": "latest" | ||
| "version": "detect" |
There was a problem hiding this comment.
@saurabhraghuvanshii makes sense but i used detect because it's recommended one, and latest forces linter to use it's newest ruleset regardless our package.json.
However if you think otherwise i will revert it, Thanks !
|
@YASHMAHAKAL did you test all the scripts that are mentioned in package.json especially ( npm run dev & npm run build ) ? |
|
Signed-off-by: YASHMAHAKAL <yvsst01@gmail.com>
Signed-off-by: YASHMAHAKAL <yvsst01@gmail.com>
Signed-off-by: YASHMAHAKAL <yvsst01@gmail.com>
Signed-off-by: YASHMAHAKAL <yvsst01@gmail.com>
Signed-off-by: YASHMAHAKAL <yvsst01@gmail.com>
Signed-off-by: YASHMAHAKAL <yvsst01@gmail.com>
Signed-off-by: YASHMAHAKAL <yvsst01@gmail.com>
a2948a4 to
a58174f
Compare
|
@saurabhraghuvanshii take a look again, i've made the fix |

Notes for Reviewers
This PR fixes #
Migrates from Babel to SWC
Signed commits