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

Make jison a dev dependency and ship less files. #94

Closed
wants to merge 1 commit into from

Conversation

realityking
Copy link

This PR makes the following changes:

  • jison becomes a dev-depency. It's not used at runtime anyway.
  • The grammar is no longer exposed on the parser.
  • Ship only files in the npm which are necessary for the package to function.

Jison pulls in quite a few dependencies so this slims down the footprint of this package quite a bit.

Not exposing the grammar on the parser makes the code a bit smaller, should be especially beneficial in the browser.

@realityking
Copy link
Author

This would also help a licensing situation for Apiary, see apiaryio/fury-adapter-swagger#168.

@dchester ping

@opichals
Copy link

@dchester Is there anything blocking this? Could this get merged?

@realityking
Copy link
Author

@dchester I've rebased this on top of current master. I'd be awesome if you could take a look.

In addition to making the package a lot smaller and potentially fixing #119, it'd also take care of the somewhat annoying warning that nomnom is deprecated that npm shows whenever jsonpath is installed.

@realityking realityking force-pushed the jison branch 2 times, most recently from 15fea8c to ae919f9 Compare February 26, 2019 22:51

Unverified

This commit is not signed, but one or more authors requires that any commit attributed to them is signed.
@pksunkara
Copy link

Can we please get a merge and publish of this @dchester?

@dchester
Copy link
Owner

I like this one -- I'll take a closer look and aim to get it merged.

@opichals
Copy link

@dchester Any news? 😄

@dchester
Copy link
Owner

dchester commented Jun 3, 2019

This is merged in spirit, with 7fbf022 which is published as version 1.0.2.

That makes jison a dev dependency, and then we point the browser field in package.json to the browser build so that webpack and similar tools can find it there. Let me know if that misses anything from this PR.

@dchester dchester closed this Jun 3, 2019
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

Successfully merging this pull request may close these issues.

None yet

4 participants