Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update Dependencies #2356

Closed
wants to merge 5 commits into from
Closed

Update Dependencies #2356

wants to merge 5 commits into from

Conversation

belcherj
Copy link
Contributor

@belcherj belcherj commented Sep 23, 2020

Fix

Updates dependencies to the latest.

Other changes:

  • Small prettier related formatting changes
  • Post CSS now requires a config file to list plugins

Test

  1. This is a massive change. The whole app needs a walk through. Testing checklist required.

Breaking changes on Major point releases:

monaco-editor-webpack-plugin
microsoft/monaco-editor-webpack-plugin@v1.9.1...v2.0.0
Will have to read code to determine breaking changes.

@typescript-eslint/eslint-plugin and @typescript-eslint/parser
https://github.com/typescript-eslint/typescript-eslint/releases/tag/v4.0.0

css-loader
https://github.com/webpack-contrib/css-loader/releases/tag/v4.0.0

  • minimum required Node.js version is 10.13.0
  • minimum required webpack version is 4.27.0
  • the esModule option is true by default
  • default value of the sourceMap option depends on the devtool option
  • icss plugin disable by default, you need to setup the modules option to enable it
  • the modules option is true by default for all files matching /.module.\w+$/i.test(filename) regular expression, module.auto is true by default
  • the modules.context option was renamed to the modules.localIdentContext option
  • default the modules.localIdentContext value is compiler.context for the module.getLocalIdent option
  • the modules.hashPrefix option was renamed to the modules.localIdentHashPrefix option
  • the localsConvention option was moved and renamed to the modules.exportLocalsConvention option
  • the getLocalIndent option should be always Function and should always return String value
  • the onlyLocals option was moved and renamed to the modules.exportOnlyLocals option
  • function arguments of the import option were changed, it is now funciton(url, media, resourcePath) {}
  • inline syntax was changed, please write ~ before the file request, i.e. rewrite url(~!!loader!package/img.png) to url(!!loader!~package/img.png)

electron
https://www.electronjs.org/releases/stable#release-notes-for-v1000

  • Changed the default value of 'enableRemoteModule' to false. #22091
  • Changed the default value of app.allowRendererProcessReuse to true, this will prevent loading of non-context-aware native modules in renderer processes. See #18397 for more information on this change. #22336
  • Fixed the positioning of window buttons on MacOS when the OS locale is set to an RTL language (like Arabic or Hebrew). Frameless window apps may have to account for this change while styling their windows. #22016

eslint-plugin-jest
https://github.com/jest-community/eslint-plugin-jest/releases/tag/v24.0.0

  • no-done-callback: no-done-callback will now report hooks using callbacks as well, not just tests
  • no-test-callback: rename no-test-callback to no-done-callback
  • recommend no-conditional-expect rule
  • recommend no-interpolation-in-snapshots rule
  • recommend no-deprecated-functions rule
  • recommend valid-title rule
  • recommend erroring for no-jasmine-globals rule
  • no-large-snapshots: no-large-snapshots runs on all files regardless of type
  • Jasmine globals are no marked as such
  • Node 10+ required

postcss-loader
https://github.com/webpack-contrib/postcss-loader/releases/tag/v4.0.0

  • minimum supported Node.js version is 10.13
  • minimum supported webpack version is 4
  • postcss was moved to peerDependencies, you need to install postcss
  • PostCSS (plugins/syntax/parser/stringifier) options was moved to the postcssOptions option, please look at docs
  • sourceMap default value depends on the compiler.devtool option
  • the inline value was removed for the sourceMap option, please use { map: { inline: true, annotation: false } } to achieve this
  • source maps contain absolute paths in sources
  • loader output only CSS, so you need to use css-loader/file-loader/raw-loader to inject code inside bundle
  • exec option was renamed to the execute option
  • the config option doesn't support Object type anymore, config.path and config.ctx options were removed
  • argument in the config for Function notation (previously config.ctx) was changed, now it contains { file, mode, webpackLoaderContext }
  • loader context in the config was renamed from webpack to webpackLoaderContext

sass-loader
https://github.com/webpack-contrib/sass-loader/releases/tag/v10.0.0-rc.0

  • loader generates absolute sources in source maps, also avoids modifying sass source maps if the sourceMap option is false

typescript
https://devblogs.microsoft.com/typescript/announcing-typescript-4-0/
No breaking changes

react-overlays
react-bootstrap/react-overlays@v3.2.0...v4.0.0

  • popperConfig longer accept object forms of modifiers, pass an array instead
  • overlay and dropdown menu injected values are different
  • overlay no longer triggers an update when placement change due to auto or flip placements
  • address feedback

@belcherj belcherj self-assigned this Sep 23, 2020
Copy link
Contributor

@dmsnell dmsnell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you summarize the changes from the updated deps here? We're probably not so concerned with 0.13.5 to 0.13.7 for regenerator-runtime but 1.9.0 to 2.0.0 for monaco-editor-webpack-plugin is more concerning.

It'd be nice to catalog all the major/breaking changes in the PR description and git commit message to make it clearer when trying to reason about this change, or when looking for an explanation why something went wrong.

If it's too big I wouldn't worry about breaking this up into separate updates. That can make git bisect easier to use as well.

  • date-fns
  • electron-fetch (what is this? is it for the auto-updater?)
  • monaco-editor-webpack-plugin
  • pretty-bytes (what is this used for?)
  • react-monaco-editor (it looks like a small update but probably any changes are significant for us to be aware of. this probably updates the monaco editor as well, so maybe we need to look at which version of monaco is updated here and list the changes from it)
  • react-overlays

@belcherj belcherj marked this pull request as draft September 23, 2020 21:14
@belcherj
Copy link
Contributor Author

  • electron-fetch: yes, it is used for the updater.
  • pretty-bytes: it is used to display progress for downloading electron updates

@belcherj
Copy link
Contributor Author

@dmsnell
Copy link
Contributor

dmsnell commented Sep 23, 2020

@belcherj thanks for posting the changesets. that's way more exhaustive than I meant to communicate when I asked for them. the points I get out of the changes are mainly this:

  • css-loader moved the ~ in inline imports from the front to the front of the filename. I don't believe we have any instances of that in our CSS even though we have used inline loaders in the JS code.
  • electron enableRemoteModule is now false by default but we have addressed this in other remote changes we've made
  • lots of minimum version bumps, different defaults, and internal changes

looks like we need some extra audit for react-overlays unless you can make sense of what's in that changeset and know how it relates to our code

@codebykat
Copy link
Member

We had a bot doing these one-by-one for a while, is it worth revisiting? I feel like it would be easier to test dependency updates one at a time as they happen.

@belcherj
Copy link
Contributor Author

@codebykat I was spending at least a day a week handling dependency updates. It was too much work.

@codebykat
Copy link
Member

I fixed the keybinding issue (microsoft/monaco-editor#102 (comment)) but still seeing one other console error:

simpleWorker.js?46cd:20 Uncaught ReferenceError: regeneratorRuntime is not defined
[...]
iterator.js?dd8a:6 Uncaught ReferenceError: regeneratorRuntime is not defined
    at eval (iterator.js?dd8a:6)
    at eval (iterator.js?dd8a:79)
    at Module.../node_modules/monaco-editor/esm/vs/base/common/iterator.js (editor.worker.js:181)
    at __webpack_require__ (editor.worker.js:20)
    at eval (lifecycle.js:11)
    at Module.../node_modules/monaco-editor/esm/vs/base/common/lifecycle.js (editor.worker.js:205)
    at __webpack_require__ (editor.worker.js:20)
    at eval (simpleWorker.js:7)
    at Module.../node_modules/monaco-editor/esm/vs/base/common/worker/simpleWorker.js (editor.worker.js:313)
    at __webpack_require__ (editor.worker.js:20)

I also rebased and removed the clipboard types from the production dependencies, seems it snuck in there again. (ref: #2122 )

@codebykat
Copy link
Member

Put the keybinding fix in over here #2546 and I'm sure I have made this need a rebase now. The regenerator runtime fix should allow us to upgrade Monaco in future.

@codebykat
Copy link
Member

Another piece of this in #2575.

@belcherj
Copy link
Contributor Author

belcherj commented Feb 4, 2021

Closing as this is too far out of date to merge in any way.

@belcherj belcherj closed this Feb 4, 2021
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.

None yet

3 participants