MiguelCastillo / Brackets-InteractiveLinter

Interactive linting for Brackets
Other
119 stars 26 forks source link

Remove third-party dependencies #160

Open StephenGregory opened 9 years ago

StephenGregory commented 9 years ago

What do you think of not storing dependencies within the repositories? I think it clearly defines the source code of this extension vs third-party software. You'll see that I'm pulling in the same versions that you have in the repository.

This is just an in-progress look at what I'm looking at.

To pull in dependencies:

$ npm install
$ gulp

Dependencies left to be removed:

Other tasks can be added to create a distributable package.

MiguelCastillo commented 9 years ago

@StephenGregory This is a fantastic PR! It will certainly help maintain dependencies. I have to go through it in details to see how you are handling JSHint and ESLint. I will try to load this one this evening and give it a try. Thank you for this PR!

StephenGregory commented 9 years ago

Great! In the meantime, if you could provide the resources for the remaining tools, I can work on pulling in those dependencies as well.

MiguelCastillo commented 9 years ago

utils is something I created a while ago... I have created belty, which has extend. We can use that to replace mixin.

reactools is an npm package.

jslint I believe I got it directly from github. Not sure, it has been a while.

StephenGregory commented 9 years ago

Thanks! With respect to reacttools.js, how did you go about minifying and concatenating the package? (through npm run build ?)

With respect to jslint, we can pull in the version with the same date stamp (although, the source code is slightly different if you do a diff between the two). Alternatively, we can try out the latest version.

Also, I'll check out belty and work on integrating that.

MiguelCastillo commented 9 years ago

So for reacttools, I installed the npm package and ran browserify on it via command line.

npm install cd node_modules/react-tools browserify main.js -s reacttools > reacttools.js

Updating to the latest jslint sounds good to me. :)

belty is on npm and the dist folder has browser bundles.

StephenGregory commented 9 years ago

Hmm. For reacttools, I am having issues reproducing the output in this repo (the output I get is similar, but has an extra ~500 lines).

Commands I ran (picking the versions you would have had available to you when reacttools.js was added to this repo:

$ npm install browserify@10.0.0
$ npm install react-tools@0.13.0
$ node_modules/browserify/bin/cmd.js main.js -s reacttools -o reacttools.js

For a quick comparison, I used wc -l reacttools.js.

Also, I think I've replaced utils.js appropriately with belty.

MiguelCastillo commented 9 years ago

@StephenGregory I tried browserifying reactools last night and I also could not get the exact same output, but it worked just fine when I tested it. I can push up the new reactools so that you can compare outputs, if you would like me to.

StephenGregory commented 9 years ago

Sure @MiguelCastillo !

MiguelCastillo commented 9 years ago

Here you go :) https://github.com/MiguelCastillo/Brackets-InteractiveLinter/pull/162

It's already merged.

StephenGregory commented 9 years ago

Perfect. However, it's still not quite what I am getting. I posted a gist to get your thoughts. Maybe it is obvious what I'm not doing.

MiguelCastillo commented 9 years ago

Yeah, it's just the order in which browserify bundled the modules. Go ahead and ignore that issue. I had browserify 10.x.x when it created the one I just checked in. I upgraded to 11.x.x and I am getting a different bundle.

I loaded your gist into my running instance of interactive linter, and it works well. Thanks for trying to track this issue down. :)

StephenGregory commented 9 years ago

Great! No problem. I used the gulpjs browserify recipe to integrate reacttools.js

Now, what is remaining is JSLint.

Edit: Where did JSLintError.js come from?

MiguelCastillo commented 9 years ago

@StephenGregory thank you so much. This is great!

MiguelCastillo commented 9 years ago

Sorry I didn't answer the question about JSLintError.js before... Didnt see it. That was created by someone I collaborated on earlier on when interactive linter was just getting started. There isn't really an npm package and really is something that we maintain ourselves. :) Feel free to dig!

StephenGregory commented 9 years ago

Ahh, gotcha! No worries. Just making sure I tracked down everything.

I added JSLint (not the latest version, the Feb 18 2013 version). I have not tested it yet, though.

I have not tested all of these changes with Brackets yet.

MiguelCastillo commented 9 years ago

@StephenGregory I pulled down your branch and when I run gulp chokidar throws some exceptions. What version of node are you running?