toji / gl-matrix

Javascript Matrix and Vector library for High Performance WebGL apps
glmatrix.net
MIT License
5.4k stars 725 forks source link

Please revert version to 3.3.0 as 3.4.0 breaks things #439

Closed huashengwang1989 closed 3 years ago

huashengwang1989 commented 3 years ago

Hi! please help revert version to 3.3.0 or bump version to 4.0.0 as many modules depending on gl-matrix is using '^'. 3.4.0 results build errors. Great appreciated!

image

Selerski commented 3 years ago

I have the same issue, a storybook with a map component breaks with this error:

Module not found: Error: Can't resolve 'gl-matrix/mat4' in '/(...)/node_modules/@math.gl/web-mercator/dist/esm'

A quick resolution would be greatly appreciated!

bilalajanjua commented 3 years ago

Yup. This must be resolved as a lot of packages are dependent on that. Mostly react-map-gl, deck.gl, nebula.gl etc.

But for the quick and temp fix, yarn resolutions can help by adding following in the package.json file.

"resolutions": {
   "gl-matrix": "3.3.0",
}
p4cm4n commented 3 years ago

Came here to post the same, can confirm it works until an official fix

Yup. This must be resolved as a lot of packages are dependent on that. Mostly react-map-gl, deck.gl, nebula.gl etc.

But for the quick and temp fix, yarn resolutions can help by adding following in the package.json file.

"resolutions": {
   "gl-matrix": "3.3.0",
}
pragyakar commented 3 years ago

Is there any solution I can use without using yarn?

Yup. This must be resolved as a lot of packages are dependent on that. Mostly react-map-gl, deck.gl, nebula.gl etc.

But for the quick and temp fix, yarn resolutions can help by adding following in the package.json file.

"resolutions": {
   "gl-matrix": "3.3.0",
}
bilalajanjua commented 3 years ago

Is there any solution I can use without using yarn?

Yup. This must be resolved as a lot of packages are dependent on that. Mostly react-map-gl, deck.gl, nebula.gl etc.

But for the quick and temp fix, yarn resolutions can help by adding following in the package.json file.

"resolutions": {
   "gl-matrix": "3.3.0",
}

Hi,

Yeah. There is another package which might help you with resolutions. I haven't tried it though, so can't guarantee.

https://www.npmjs.com/package/npm-force-resolutions

ldegen commented 3 years ago

@bilalajanjua thx man, you're my saviour. :-)

toji commented 3 years ago

I'll unpublish 3.4.0 for now. It looks like the top-of-tree has made changes (see https://github.com/toji/gl-matrix/pull/432#issuecomment-853225900) that simply aren't going to be backwards compatible with every build system so I'll need to do a major version bump if we're going to publish any new updates. Might not be a bad time to see if there's any other cleanups that need to be done while I'm at it.

toji commented 3 years ago

NPM won't let me unpublish 3.4.0 since apparently there's already some dependencies on it 😮 so I've marked it as deprecated instead. It's not clear to me that will resolve the issue with people who have a dependency on ^3.3.0, though, so I'm looking into publishing a new version using an older commit in the repo before the backward compat breaking changes were introduced.

If anyone happens to be able to confirm that the deprecation DID fix the problem for you, though, I'd appreciate hearing it!

bilalajanjua commented 3 years ago

NPM won't let me unpublish 3.4.0 since apparently there's already some dependencies on it 😮 so I've marked it as deprecated instead. It's not clear to me that will resolve the issue with people who have a dependency on ^3.3.0, though, so I'm looking into publishing a new version using an older commit in the repo before the backward compat breaking changes were introduced.

If anyone happens to be able to confirm that the deprecation DID fix the problem for you, though, I'd appreciate hearing it!

I guess deprecation will work. I'm gonna quickly verify now.

bilalajanjua commented 3 years ago

NPM won't let me unpublish 3.4.0 since apparently there's already some dependencies on it 😮 so I've marked it as deprecated instead. It's not clear to me that will resolve the issue with people who have a dependency on ^3.3.0, though, so I'm looking into publishing a new version using an older commit in the repo before the backward compat breaking changes were introduced.

If anyone happens to be able to confirm that the deprecation DID fix the problem for you, though, I'd appreciate hearing it!

Unfortunately, It seems deprecation doesn't work.

If we simply run:

yarn add gl-matrix

It installs 3.4.0 version although it's deprecated and also shows the deprecation warning.

image

And if we try to install other packages which are using gl-matrix as a dependency and have set version as:

"gl-matrix": "^3.0.0"

then it is again resolves to the latest version which is 3.4.0.

image

image

So I guess, for now the only option is to republish the package with an older commit to resolve this issue.

toji commented 3 years ago

Thanks for checking on that, @bilalajanjua. I've just published v3.4.1, which rolls in a few new fixes but avoids the backwards compat breaking change and should resolve the issue with the deprecated package taking priority. Please give it a try and let me know if you run into any additional problems!

JMariano88 commented 3 years ago

It works for me now.. good

JMariano88 commented 3 years ago

I used "npm i gl-matrix@3.3.0" to restore gl matrix version, I think it's work fine

bilalajanjua commented 3 years ago

Thanks for checking on that, @bilalajanjua. I've just published v3.4.1, which rolls in a few new fixes but avoids the backwards compat breaking change and should resolve the issue with the deprecated package taking priority. Please give it a try and let me know if you run into any additional problems!

No problem Brandon.

I tried with the version 3.4.1, but while building the react app with @math.gl as a dependency. I still get the same error i was getting with 3.4.0.

image

@math.gl is importing the vec2 module as follows, but I guess we can't import it like this in v3.4.1:

import * as vec2 from 'gl-matrix/vec2';

Instead, it has to be imported like this as per new version:

import { vec2 } from 'gl-matrix';

So i guess, version 3.4.1 still doesn't fix the problem.

toji commented 3 years ago

Thank you again for helping debug that. I'm not clear on what would cause that difference between 3.3.0 and 3.4.1, as it should have just been some minor fixes and additions to the functions themselves, not how the build was published. I'll continue investigating.

pragyakar commented 3 years ago

Can confirm that 3.4.1 did not solve the issue for me either.

image

image

bilalajanjua commented 3 years ago

Thank you again for helping debug that. I'm not clear on what would cause that difference between 3.3.0 and 3.4.1, as it should have just been some minor fixes and additions to the functions themselves, not how the build was published. I'll continue investigating.

No worries.

Yeah it seems bundling logic also got changed in one of those fixes. I'll also try to find some time to investigate the cause.

donmccurdy commented 3 years ago

Just an FYI for anyone using glTF-Transform, gltfjsx, or React Three Fiber — I've pinned @gltf-transform/core and @gltf-transform/functions to ~3.3.0 for the time being. This will indirectly pin gltfjsx and R3F to the older gl-matrix version as well.

toji commented 3 years ago

Okay, I suspect that the issue is that I wasn't building with the exact npm dependencies from v3.3.0, and that changed the output format in a breaking way. I've tried to address that, and the changes to dist/gl-matrix look far less disruptive now, but I'd like to try and verify before pushing out yet another npm release.

Is anyone in a position to try the branch https://github.com/toji/gl-matrix/tree/v3.4.x against code that has started failing since 3.4.0 was pushed and verify if it works or fails for you? (Should be able to set the dependency to git://github.com/toji/gl-matrix.git#1841163019f89a6ff6376327948ea14e29bbfc95 temporarily to test, I think?)

bilalajanjua commented 3 years ago

@toji I am verifying that.

bilalajanjua commented 3 years ago

Okay, I suspect that the issue is that I wasn't building with the exact npm dependencies from v3.3.0, and that changed the output format in a breaking way. I've tried to address that, and the changes to dist/gl-matrix look far less disruptive now, but I'd like to try and verify before pushing out yet another npm release.

Is anyone in a position to try the branch https://github.com/toji/gl-matrix/tree/v3.4.x against code that has started failing since 3.4.0 was pushed and verify if it works or fails for you? (Should be able to set the dependency to git://github.com/toji/gl-matrix.git#1841163019f89a6ff6376327948ea14e29bbfc95 temporarily to test, I think?)

Nope, @toji . That doesn't work as well. Getting the same error as before (https://github.com/toji/gl-matrix/issues/439#issuecomment-934582146).

toji commented 3 years ago

Ugh, thanks for checking.

One last thing, if you don't mind (thank you so much for your help so far!) Can you try git://github.com/toji/gl-matrix.git#93d9f756d7bdc32d163f36d0b8cad74a6a795a98? That's the same commit as v3.3.0 was built from with just a straight up rebuild. I'd like to know if that works, because if not it means I'm not able to rebuild a compatible release. Which... would not be great.

bilalajanjua commented 3 years ago

With 3.4.2, here's the intellisense i get:

image

And here's the package tree:

image


With 3.3.0, here's the intellisense i get:

image

And the package tree is as follows:

image

I noticed that there is no dist folder as well. So something is changed in bundling logic.

bilalajanjua commented 3 years ago

Ugh, thanks for checking.

One last thing, if you don't mind (thank you so much for your help so far!) Can you try git://github.com/toji/gl-matrix.git#93d9f756d7bdc32d163f36d0b8cad74a6a795a98? That's the same commit as v3.3.0 was built from with just a straight up rebuild. I'd like to know if that works, because if not it means I'm not able to rebuild a compatible release. Which... would not be great.

Sure @toji, On it.

bilalajanjua commented 3 years ago

@toji This seems fine. It generates the same tree that i sent earlier for version 3.3.0. Although it's still under dist folder but i guess with npm publish, dist folder's content will be moved to the root directory. If yes, then it will actually solve the problem.

Here's the tree:

image

toji commented 3 years ago

Thanks again @bilalajanjua. I've published v3.4.2 to NPM and deprecated v3.4.1. This is just the v3.3.0 release with two new functions that will benefit WebGPU developers in the future. Verification that it's working with other projects would be welcome!

(And I'm going to have to set aside some time to do some serious restructuring of the current build system for an eventual 4.0.0 release. I'm very distressed at how brittle it's ended up being, though to be fair it's also been allowed to bit-rot for a while.)

bilalajanjua commented 3 years ago

Thanks again @bilalajanjua. I've published v3.4.2 to NPM and deprecated v3.4.1. This is just the v3.3.0 release with two new functions that will benefit WebGPU developers in the future. Verification that it's working with other projects would be welcome!

(And I'm going to have to set aside some time to do some serious restructuring of the current build system for an eventual 4.0.0 release. I'm very distressed at how brittle it's ended up being, though to be fair it's also been allowed to bit-rot for a while.)

@toji I am afraid, v3.4.2 when installed brings dist folder with it and that's why

import * as vec2 from 'gl-matrix/vec2';

doesn't work and

import * as vec2 from 'gl-matrix/dist/vec2';

works! (which is not expected :worried:)

I thought bundling actually takes away dist folder and packs only the content inside it as it was previously done in v3.3.0.

toji commented 3 years ago

Oh, I may have misunderstood something fairly fundamental about how @stefnotch was previously publishing the package. Let me verify something...

donmccurdy commented 3 years ago

In package.json there are mappings from the import names to the files in dist/. Bundlers, and Node.js versions >=v13, should be able to resolve these paths. Related to some issues for TypeScript users: https://github.com/toji/gl-matrix/pull/390, https://github.com/toji/gl-matrix/issues/423. I'm not sure what would happen if your JS code is being compiled to CommonJS before running in Node.js in this case.

https://github.com/toji/gl-matrix/blob/0bca31acb13cdaa6eaa2bd615b053d9fdd4e7b72/package.json#L9-L21

toji commented 3 years ago

Well crap. Turns out that the previous npm packages were being published FROM the dist folder, whereas I was publishing from the root.

...sigh

v3.4.3 has been published now (sure, let's just burn through ALL the 3.4.x's why don't we.) Let's give that a try.

(I apologize that this has been so much back and forth. I include gl-matrix in my own projects as a ES module and so the depenencies work out differently.)

stefnotch commented 3 years ago

Oh dearie, I'm very sorry for not properly documenting everything. If I recall correctly, I was publishing the package by doing

npm install
cd dist
npm publish

I'm not entirely sure why I had to cd into the dist directory for everything to work.

It's also quite possible that one of the build system changes that I merged in was backwards incompatible.

toji commented 3 years ago

No, this really isn't your fault. If anything it's entirely due to my lack of activity on the project lately. I can see why the builds were being published that way now, it just wasn't what I remembered/expected based on some of the other NPM packages I've had a more hands-on involvement in lately.

bilalajanjua commented 3 years ago

Thanks guys @toji @stefnotch @donmccurdy 🚀

I can confirm v3.4.3 works perfectly fine just like 3.3.0

P.S. These last few comments gave me an idea on how to publish a package without that dist folder. So yeah thanks for that. 😄

toji commented 3 years ago

Hallelujah! Thank you for taking the time to help me set that right, it's been invaluable.

And now that I know what went wrong I can push out fixes more confidently in the future. Again, though, it may be best if I leave any big changes to a potential 4.0.0 just to avoid this whole song and dance.

InCogNiTo124 commented 2 years ago

I'm having trouble with 3.4.3 :frowning_face: but I have a specific setup because I'm developing in a development docker container:

I'm trying to setup this folder:https://github.com/davidwparker/programmingtil-webgl/tree/master/0063-3d-cube-mouse-rotate

I've obtained gl-matrix:3.4.3 from npm by npm install -g gl-matrix and copying /usr/local/lib/node_modules/gl-matrix/gl-matrix-min.js into my folder:

.
├── css
│   └── styles.css
├── gl-matrix-min.js
├── index.html
├── index.js
├── libs
│   ├── glUtils.js
│   └── uiUtils.js
├── README.md
├── shaders
│   ├── fragment.glsl
│   └── vertex.glsl
└── signals.min.js

and I edited the index.html to fix the paths

however, running http-server (as is suggested by readme) from this folder results with:

Uncaught ReferenceError: mat4 is not defined
    at draw (index.js:125:15)
    at Object.state.animation.tick (index.js:97:7)
    at animate (index.js:100:21)
    at main (index.js:62:5)
    at callback (index.js:54:43)
    at h.execute (signals.min.js:9:20)
    at e.dispatch (signals.min.js:13:106)
    at e.dispatch (signals.min.js:8:368)
    at Object.checkForComplete (glUtils.js:228:27)
    at processShader (glUtils.js:241:18)

Everything is correctly loaded, no 404's.

edit: 3.3.0 still doesn't work as well, though, I may have bumped this unnecessarily

InCogNiTo124 commented 2 years ago

BTW I belive you haven't released a git 3.4.3 release :sweat_smile: