Eslint + npm #410
Eslint + npm #410
Conversation
|
I really don't want to introduce browserify. I'd much prefer switching to node modules and using rollup. |
|
@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
force-pushed
the
eslint-npm
branch
2 times, most recently
from
ad28bf2
to
4942c44
Compare
|
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 |
|
It will be nice to get this merged |
package.json
Outdated
| "files": [ | ||
| "LICENSE.BSD", | ||
| "README.md", | ||
| "bin", | ||
| "escodegen.js", | ||
| "escodegen.browser.min.js", | ||
| "escodegen.browser.js", | ||
| "package.json" | ||
| ], | ||
| "version": "1.14.3", | ||
| "engines": { | ||
| "node": ">=4.0" |
There was a problem hiding this comment.
Per estools/esquery#96 (comment) and context, @michaelficarra was inclined for the project to keep engines and only bump if the project is known to fail. I was planning on also adding a PR for babel support on top of this as should allow that backward support (but had been informed there was too much in this PR originally to include the Rollup/Babel routine). So I can bump engines if desired, or add Babel to provide backward-support, but if the latter, I presume Babel is still desired in another PR.
There was a problem hiding this comment.
Btw, I've rebased on master, and also noted in my intro comment that I've also added these changes on top of the previously mentioned ones:
- npm: Restore optionator to a regular dep. (used in published binary file)
- npm: Drop unused semver, minimist
There was a problem hiding this comment.
source-map 0.7.x drops support for Node < 8.
There was a problem hiding this comment.
Ok, but source-map has been a part of optionalDependencies.
- Update: Use new SourceMapConsumer API in test - Build: Provide browserified builds with npm package - Travis: Drop 4, 6, 8; Add 10, 12, 14; 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
- npm: Update package-lock version
- npm: update devDeps. - npm: Update package-lock version
|
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... |
|
@michaelficarra : Might I get a review for this? Would like to supply a native ESM/Rollup PR after this if this is ok. |
.editorconfigbugs,keywords, change frommaintainerstoauthors/contributorsoptionatorto a regular dep. (used in published binary file)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)