Outdated dependencies. Collecting feedback

[Akiyamka]

Feature request

Currently repo have a bunch of outdated deps. Let's refresh it.

Background

Before i make a merge request to close this issue, I would like to know the opinion of maintainers on this repo, perhaps now is not the right time to do this, or there are other reasons why it should not be done

As i see a large number of obsolete dependencies in the project:

• creates security flaws
• Makes existing examples irrelevant
• misses fresh bug fixes and performance improvements
• dependabot flooded with spam with upgrade requests

To Do List

(separate mr for every step)

• [x] Upgrade all deps to last minor version
• [x] Migrate to vite and vitest ~~Migrate to react-map-gl 7~~
• [x] Migrate to react 17
• [ ] Migrate to react 18

[ibgreen]

This sounds good.

• I hear there are still some problems with deck.gl and React 18. Maybe start with upgrading to React 17?
• Nothing wrong with webpack 5, though it seems we are moving away from webpack in other repos, towards an esbuild/tsc combo. For examples I have been moving to vite, see e.g. luma.gl examples. Cleaner config and faster builds.

@pessimistress

[Akiyamka]

I hear there are still some problems with deck.gl and React 18. Maybe start with upgrading to React 17?

At first glance works without any problems, what should I pay attention to?

Nothing wrong with webpack 5, though it seems we are moving away from webpack in other repos, towards an esbuild/tsc combo. For examples I have been moving to vite, see e.g. luma.gl examples. Cleaner config and faster builds.

The webpack was offered as a more conservative option, but if you don't mind using vite, that's much better. In that case mayby also replace jest with vitest? Vitest have compatible api, but the integration is better

[ibgreen]

You can look for React 18 in the deck.gl discussions / issues. It seemed to me by browsing through some of these discussion that React 18 can cause rendering sync issues between deck.gl and basemaps, my guess is that it could be due to aggressive under-the-hood-optimizations in React 18 that do not exhibit any problems in normal DOM applications. I suspect the risk is that if you force apps to upgrade to 18, you may leave them stuck without downgrade option should such rendering issues arise.

As for you other questions, nebula.gl has always had a slightly different build setup than the standard for other vis.gl repos. As long as this is done by a committed maintainer and the tool choices are not too obscure, I don't see what that couldn't continue, i.e. we can let nebula.gl be a place where we allow some experimentation with tooling, that other vis.gl repos can take inspiration from.

[Akiyamka]

Great, in that case let's do it!

[omasback]

You can always allow multiple major versions of react like so:

"peerDependencies": {
  "@types/react": "^17.0.0 || ^18.0.0",
  "react": "^17.0.0 || ^18.0.0",
  "react-dom": "^17.0.0 || ^18.0.0"
},

[omasback]

As for react-map-gl 7, i think they said it would never be compatible with react-map-gl-draw and to use mapbox-gl-draw instead. unfortunately mapbox-gl-draw seems to be abandoned, with no new releases in almost two years. Hopefully the community can consolidate around one of the two libraries.

https://github.com/visgl/react-map-gl/issues/1892

[Akiyamka]

Thank you @omasback, look like migration to react-map-gl 7 is something we can't do now.

As an alternative solution, you can work with the map api directly without using it react wrappers (essentially all you need from react is a ref on the dom node);

Possible implementation:

class MapAPI {
  constructor(mapEngine) {
    this.engine = mapEngine;
  }
  
  init(element) {
    this.map = new this.engine.Map({
      container: element,
      style: 'mapbox://styles/mapbox/streets-v12', // style URL
      center: [-74.5, 40],
      zoom: 9 
     });
  }
}

export const map = new MapAPI(mapboxgl);

export function MyMap() {
  const mountPointRef = useRef(null);
  useLayoutEffect(() => {
    map.init(mountPointRef.current)
  }, [mountPointRef])
  
  return <div ref={mountPointRef}></div>
}

[Komzpa]

With #862 being merged bumping react to 17 and implementing vite/vitest migration, I believe this issue can be closed.

I would also appreciate if the release could be cut, there's quite a bit of enhancements in the master branch and we don't have any more changes lined up from @Konturio in this iteration.

[ibgreen]

I would also appreciate if the release could be cut

@Akiyamka - you has npm publish privileges from @konturio side. Want to give it a try?

https://www.npmjs.com/settings/nebula.gl/members

[Akiyamka]

@ibgreen Do you have any doc describing the release process? When I try, I get error about failed push to protected branch:

$ yarn publish-prod
<...>
lerna info git Pushing tags...
lerna WARN gitPush remote: error: GH006: Protected branch update failed for refs/heads/master.        
lerna WARN gitPush remote: error: At least 1 approving review is required by reviewers with write access.        
lerna WARN gitPush To github.com:uber/nebula.gl.git
lerna WARN gitPush  ! [remote rejected] master -> master (protected branch hook declined)
lerna WARN gitPush  ! [remote rejected] v2.0.0-alpha.0 -> v2.0.0-alpha.0 (atomic transaction failed)
lerna WARN gitPush error: failed to push some refs to 'github.com:uber/nebula.gl.git'

[apalmer0]

Any updates on this?