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

Support for browserify 5? #78

Closed
dsferruzza opened this issue Jul 24, 2014 · 18 comments
Closed

Support for browserify 5? #78

dsferruzza opened this issue Jul 24, 2014 · 18 comments

Comments

@dsferruzza
Copy link

Hi!

The current package.json prevents browserify-shim to work with browserify 5: "browserify": ">= 2.3.0 < 5".
But now the latest stable is 5.1.0 (5.0.0 is less than a day old).

Is browserify-shim compatible with browserify 5?

@jgoux
Copy link

jgoux commented Jul 25, 2014

+1, I got the issue this morning.

@ghost
Copy link

ghost commented Jul 25, 2014

+1 for this.
I am unable to use the angulpify generator of @jgoux which i discovered in reddit !!

@jgoux
Copy link

jgoux commented Jul 25, 2014

@diwalak Actually you can, the error is more a warning, everything is installed. ^^

@thlorenz
Copy link
Owner

We can update the peer dependency in the package.json in order to remove this warning. I'll do that some time today.

@thlorenz
Copy link
Owner

I tried to upgrade browserify to 5.8 and remove peerdep upper limit along with that.
However some tests are failing, so I cannot publish as is. Seems like the new browserify is not fully backward compat.

If anyone wants to have a look and get the tests to pass, I'd appreciate it. Until then I cannot publish the new version. I pushed the updated version to browserify-5.8 branch.

@jacobsvante
Copy link

Have you had time to look any more into this @thlorenz?

@thlorenz
Copy link
Owner

No, seems like browserify broke backwards compatibility.
I'm currently swamped, but am hoping for someone who is more aware of what changed in browserify to be able to fix the tests and submit a PR so we can properly publish a new version.

@feross
Copy link

feross commented Aug 1, 2014

This commit may be helpful: browserify/browserify@d6fc91b

@thlorenz
Copy link
Owner

thlorenz commented Aug 1, 2014

Thanks @feross I will try to have a look as soon as I come free, but in the meantime if anyone wants to take this on I'd greatly appreciated it :)

@javoire
Copy link

javoire commented Aug 4, 2014

+11111 :)

@webgefrickel
Copy link

Yup, would be very nice, if anyone can look into this. I tried to get browserify/watchify with browserify-shim working, but watchify now requires browserify 5.5+

@thlorenz
Copy link
Owner

thlorenz commented Aug 7, 2014

Just to be clear all you get is a warning during npm install right? Aside from this everything should be working fine. So you should be able to use browserify-shim even with later versions of browserify.

I just am trying to refrain from upgrading the peer dep before all the tests are passing with the new browserify version.
AFAIK the changes in the browserify API only affect how the tests work since they depend on things like having a require returned when running the bundle.

@webgefrickel
Copy link

This is the warning I get (having installed browserify in version 5.9.1 (current) already):

❯ npm i --save-dev browserify-shim
npm ERR! peerinvalid The package browserify does not satisfy its siblings' peerDependencies requirements!
npm ERR! peerinvalid Peer browserify-shim@3.6.0 wants browserify@>= 2.3.0 < 5
...

@thlorenz
Copy link
Owner

thlorenz commented Aug 7, 2014

Yeah, but that doesn't prevent you from finishing the install, i.e. no one should be blocked on this.
As I said, totally swamped ATM, so will have to address this later if no one else steps up to the challenge.

@osama-lionheart
Copy link

@thlorenz, actually there are some side-effects, that I think are caused by upgrading browserify to v5.9.1 which I discovered while experiencing a similar issue to #40. you can see a repro of the problem by using @jkymarsh's repo that he provided in #40 as a proof of a working example, but unfortunately it doesn't work right now with browserify v5.9.1

@AlexRiedler
Copy link

After much thinking and twiddling with code and tests, no source code changes are needed for browserify-shim to work.

The current issue is the tests are failing as a result of inspecting the internals of browserify (that have changed); or using a no longer supported feature.

Also wrapping the browserify function / bundle does not work as it use to; but using it just as a transform works perfectly.

@thlorenz
Copy link
Owner

That's good news although @osama-lionheart was reporting regressions, so not sure what to think.

As long as it's still useful as a transform that shouldn't be a problem.
What needs to be done in order to fix the tests?

@thlorenz
Copy link
Owner

Fixed in v3.7.

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

No branches or pull requests

9 participants