Eslint + npm - Githubissues

brettz9 commented 4 years ago

Breaking change: Remove bower-registry-client build (bower deprecated)
Linting: Apply some basic fixes
Update: Use new SourceMapConsumer API in test
Build: Provide browserified builds with npm package
Travis: Drop 4, 6, 8; Add 12, 14, 16, 17; check build
Maintenance: Add .editorconfig
Docs: Use fenced code blocks in README (for syntax highlighting)
npm: Add bugs, keywords, change from maintainers to authors/contributors
npm: Restore optionator to a regular dep. (used in published binary file)
npm: Drop unused semver, minimist
npm: Bump deps. (estraverse, optionator, optional source-map potentially breaking) and devDeps.
npm: Drop bluebird in favor of ES Promises
npm: Use more recently maintained browserify + uglifyify
npm: Replace linting and testing scripts in Gulpfile with npm scripts

~FWIW, I've added a commit for adding a babel routine against the new code in case any of the syntax was not fully supported back to Node 4, though I can undo that if final testing determines it is not necessary.~ (Planning to add to a later PR)

michaelficarra commented 4 years ago

I really don't want to introduce browserify. I'd much prefer switching to node modules and using rollup.

michaelficarra commented 4 years ago

@brettz9 This PR does too many things to do a proper review. Can you break it down into smaller PRs? You don't need one per commit, but please at least have everything in each PR related.

brettz9 commented 4 years ago

I've cut back to avoid a lot of the linting changes (which I can add to a subsequent PR). There are still a number of different items in the PR, but I think it should be a lot easier to review now. Let me know if you need it subdivided further. Most of the changes should be related now to updating the deps and some minor packaging additions (metadata or added config files).

While I know you weren't keen on supporting browserify elsewhere, commonjs-everywhere was not working with the current set-up (complaining about a reserved word in the updated source-map). I intend to submit a Rollup PR later, but doing that now would necessitate a great deal more changes, making it a lot harder to review at once (e.g., by removing from an IIFE to exports, the indent is changed, causing a lot of diff noise).

sanex3339 commented 4 years ago

It will be nice to get this merged

brettz9 commented 3 years ago

Hi,

Anything else you need for this? I've got another PR for Rollup/Babel lined up after this, but keeping it more focused for this PR...

brettz9 commented 2 years ago

@michaelficarra : Might I get a review for this? Would like to supply a native ESM/Rollup PR after this if this is ok.

estools / escodegen

Eslint + npm #410