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

chore(deps): December 2023 bump #2234

Merged
merged 11 commits into from Dec 22, 2023
Merged

Conversation

Desplandis
Copy link
Contributor

@Desplandis Desplandis commented Dec 5, 2023

Description

Update all dependencies (when possible). I listed one major issue due to license change as well as minor issues questioning the necessity of certain dependencies or packaging problems upstream.

Peer dependencies

@mgermerie @jailln Since peer dependencies tie users of the library to those versions, shall we tag the commits as features?

  • three (0.154.0 to 0.159.0): Breaking changes given by the migration guide. Possible issues:
  • proj4 (2.9.0 to 2.9.2): No breaking changes.

Dependencies

Those are the packages with a new version available:

  • @loaders.gl/las (3.4.4 to 4.0.4): No API changes.
  • @mapbox/mapbox-gl-style-spec (13.28.0): Will not update due to the fact this is the latest version with a FOSS license (ISC). This is a huge red-flag, shall we consider moving to @maplibre/maplibre-gl-style-spec? What are the differences between those two?
  • @tmcw/togeojson (5.6.2 to 5.8.1): Follows semantic versioning, no breaking changes.
  • @tweenjs/tween.js (18.6.4): Cannot update to 21.0.0 due to error ERR_REQUIRE_ESM. They distribute a commonJS module but due to its .js extension and their package.json treating js files as ES modules causing this issue... This could be fixed on their side by changing the extension of their commonJS module to .cjs. Does not seems to be addressed upstream yet (see CJS entry point can't be loaded in Node.js tweenjs/tween.js#649 and Fix CJS entry point tweenjs/tween.js#659).
  • regenerator-runtime (0.13.11 to 0.14.0): No breaking changes. However all browsers support async functions/await statements since April 2017 (no IE11), do we still need this dependency (would need to update our babel target)?

Those packages may not be needed anymore:

  • text-encoding-utf-8: Used to polyfill TextDecoder in our code-base and is natively supported by all major browsers and Node.js since 2017 (with the exception of Edge which added support in 2020). Do we still need this dependency? Note that this can break our test-case since Node.js imports this differently (in util module).

Developer dependencies

Those packages upgrades introduce no breaking change:

  • chalk.
  • chart.js.
  • core-js.
  • https-proxy-agent.
  • marked
  • node-fetch: Update to latest 2.x.x version. Cannot update to 3.x.x since it's now a ESM only package but the 2.x.x branch is still maintained.
  • replace-in-file.
  • typescript.
  • webpack and the following loader: babel-loader.
  • whatwg-fetch: Used to polyfill the fetch API in web environment (supported by all browsers > 2016) in our webpack bundle. This may not be necessary anymore.

Those packages upgrades introduce a breaking change for our use-cases:

  • conventional-changelog-cli: Now requires a node >= 16 environment. Fix issue when the changelog of all versions was inserted in the changelog.md (only the last release is now added).
  • eslint and the following plug-ins: eslint-import-resolver-webpack and eslint-plugin-import. Note that the upgrade introduce a change in the if-else indent rule, causing a linting issue in Ellipsoid.
  • puppeteer: Use new headless mode using chrome (instead of chromium).

This package introduces coverage regression and will not be updated:

  • @babel/cli, @babel/register and the following plug-ins: @babel/plugin-transform-runtime and @babel/preset-env. Cause regression in coverage.

Notes to reviewer

I didn't had access to pre-1.0 3D Tiles files to test lighting issues and edits to LegacyGLTFLoader. Maybe @jailln you could test it on your proprietary datasets ?

@Desplandis Desplandis force-pushed the bump/dec23 branch 2 times, most recently from 5a7e20a to 11cc47a Compare December 7, 2023 14:49
@Desplandis Desplandis self-assigned this Dec 8, 2023
@Desplandis Desplandis added chore 🚚 dependencies Pull requests that update a dependency file labels Dec 8, 2023
@Desplandis Desplandis force-pushed the bump/dec23 branch 2 times, most recently from 37f8396 to f305cef Compare December 8, 2023 14:58
@mgermerie
Copy link
Contributor

@mgermerie @jailln Since peer dependencies tie users of the library to those versions, shall we tag the commits as features?

I think this is a good idea. Bumping them sometimes grants users with new features introduced in their new versions. On another point, I often pay more attention to the "feature" section of a changelog than the "workflows and chores" (or equivalent name). So based on my own experience, it would make peer dependencies update more visible. However this has yet to be confirmed by other user experiences.

@Desplandis Desplandis force-pushed the bump/dec23 branch 3 times, most recently from fd0f976 to 2edf9b8 Compare December 11, 2023 13:03
Notes:
- The lighting model has been updated in r155.
  Light intensity may requires much higher intensity values.
  See https://discourse.threejs.org/t/updates-to-lighting-in-three-js-r155/53733
- AnimationUtils.arraySlice() has been removed in r157.
  All references in LegacyGLTFLoader have been replaced by TypedArray#slice()
- Quaternions are now expected to be normalized in r158.
- webpack: 5.87.0 to 5.89.0
- babel-loader: 9.1.2 to 9.1.3
- eslint: 8.43.0 to 8.55.0
- eslint-import-resolver-webpack: 0.13.2 to 0.13.8
- eslint-plugin-import: 2.27.5 to 2.29.0

Notes:
- Updated linting rules implies fixing new warnings raised in Ellipsoid
  and WebXR
- chalk: 5.2.0 to 5.3.0
- chart.js: 4.3.0 to 4.4.1
- core-js: 3.31.0 to 3.34.0
- https-proxy-agent: 7.0.0 to 7.0.2
- marked: 5.1.0 to 11.0.1
- node-fetch: 2.6.11 to 2.7.0
  Note that we cannot update to version `>= 3.0.0` since it's now a ESM
  only package. However, the 2.x.x branch is still maintained.
- replace-in-file: 7.0.1 to 7.0.2
- typescript: 5.1.6 to 5.3.3
- whatwg-fetch: 3.6.2 to 3.6.19
- Switch to new headless mode (which uses chrome instead of chromium)
- Add explicit button parameter to mouse.up (which is now mandatory)
- Requires node >= 16
- Fix issue when the changelog of all versions was inserted in the
  changelog.md (only the last release is now added).
@Desplandis Desplandis marked this pull request as ready for review December 12, 2023 16:20
@Desplandis
Copy link
Contributor Author

@jailln @mgermerie Ready for review, I added a "Notes to reviewer" to my initial message. Unfortunately, I dont't have access to 3D Tiles with old gltfs (version = 1.0).

@mgermerie
Copy link
Contributor

I also think we could remove regenerator-runtime and text-encoding-utf-8 - perhaps in a future PR ?

@jailln jailln self-assigned this Dec 19, 2023
Copy link
Contributor

@jailln jailln left a comment

Choose a reason for hiding this comment

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

Thanks for the bump and the digest :)

  • Regarding @mapbox/mapbox-gl-style-spec, can you open a dedicated issue please?
  • I agree that we should remove regenerator-runtime, text-encoding-utf-8 and whatwg-fetch especially since we will update our babel target soon.
  • I tested with 3D tiles pre-1.0 and with gltf 1.0, everything works fine

@Desplandis
Copy link
Contributor Author

Opened an issue for mapbox and a PR for removing unecessary dependencies.
Will now merge this PR.

@Desplandis Desplandis merged commit 5a6c7e3 into iTowns:master Dec 22, 2023
9 checks passed
@Desplandis Desplandis deleted the bump/dec23 branch December 22, 2023 10:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore 🚚 dependencies Pull requests that update a dependency file
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants