indexeddbshim / IndexedDBShim

A polyfill for IndexedDB using WebSql
Other
968 stars 191 forks source link

When running npm run build-node dist/indexeddbshim-node.js changes #291

Closed erikvold closed 7 years ago

erikvold commented 7 years ago

When I clone this repo, run npm install, then npm run build-node then git status will output:

    modified:   dist/indexeddbshim-node.js
    modified:   dist/indexeddbshim-node.min.js
    modified:   dist/indexeddbshim-node.min.js.map

this is the diff https://github.com/axemclion/IndexedDBShim/compare/master...erikvold:run-build-node

shouldn't there be no change?

Using node 7.5.0 npm 4.1.2

erikvold commented 7 years ago

@brettz9 ping

brettz9 commented 7 years ago

I'll try to start over, removing my node_modules and reinstalling as well as incorporate your changes and then retest.

Btw, I'm still pretty actively working on things (though ATM working on our new dependency, typeson-registry), trying to clear up or address the remaining W3C failing tests, so though I try ensuring everything is sound (at least with the W3C tests though often all tests) before pushing to master, there ought to be more fixes on the way in the shorter term and more complete testing when that is all complete.

But yours ought to be a very helpful fix to have in the interim especially to the extent it is blocking others from testing.

brettz9 commented 7 years ago

Could you try this again for me? I wonder whether the recent changes in master should fix this.

erikvold commented 7 years ago

I updated to master removed the node_modules subdirectory and ran npm install && npm run build-node again and got https://github.com/axemclion/IndexedDBShim/compare/master...erikvold:run-build-node-ii

brettz9 commented 7 years ago

Thanks for checking... Hmm... I still want to take a closer look. When done with a current PR, I may add npm shrinkwrap to ensure we have all the same dependencies. I also saw that such an issue with varying browserify order if basedir were not supplied so I might try setting that and see if that may resolve any other such relative path problems.

brettz9 commented 7 years ago

Looking at the diff for your run-build-node-ii branch, it looks as expected--it includes some more recent updates to the typeson repo. (Our references to particular repos in npm (e.g., master branch) without a version may turn up different code for now. Though I have commit access to those repos (the current ones are my forks), I am not ready to either stabilize on a version and push back upstream or publish my own fork, so it may be a little inconsistent for now though. However, it is part of my final proposed goals for version 3.0.0 at https://github.com/axemclion/IndexedDBShim/issues/262#issuecomment-254413002 to specify stable versions, and on my next PR, I'll even try to remember to shrinkwrap it so that will enforce stable versions for the time-being to facilitate testing such as you are doing.)