esutils icon indicating copy to clipboard operation
esutils copied to clipboard

Esm eslint

Open brettz9 opened this issue 5 years ago • 6 comments

Builds on #33.

  • Breaking change: Enhancement: Add native ESM distribution, with exports property in package.json
  • Refactoring: Add Rollup/Babel/Terser
  • Build: Have generator file produce consumable source files directly rather than logging to console
  • Linting: Switch from JSHint to ESLint (using rules, e.g., indent and prefer-const, in place on other estools project)
  • Testing: Convert coffeescript test files to ES6
  • Testing: Add nyc
  • Testing: Check unmatched high surrogates and full coverage for AST expressions (further true isExpression and isStatement's and false isProblematicIfStatement), bringing to 100% coverage
  • Travis: Drop previous versions for 12, 14, 16

brettz9 avatar Apr 29 '20 11:04 brettz9

Rather than using rollup, why not use “exports” which prevents require or import of files you don’t want to be in your public api?

ljharb avatar Apr 29 '20 15:04 ljharb

As you may have been indicating yourself, exports is itself a breaking change itself of sorts, given how it may restrict access to formerly accessible module-based paths.

I like adding Rollup as it offers the ability to provide a version which can work in ESM-supporting browsers without build steps. I'd personally be cool adding exports in addition, having the potential to minimize future breaking changes.

brettz9 avatar Apr 30 '20 10:04 brettz9

Btw, though it doesn't look like it should be an issue in this repo, and shouldn't be too much of a big deal in the other estools repos (except impacting instanceof checks for exported classes), there is a potential concern for the "Dual Package Hazard" in estraverse (which has non-isolated state in its configuration of Syntax, VisitorKeys, and VisitorOption) and escodegen (which has non-isolated state for its options).

This might call for its own breaking change to isolate state (preferably the ESM-based approach indicated at https://nodejs.org/api/esm.html#esm_approach_2_isolate_state ).

brettz9 avatar Apr 30 '20 10:04 brettz9

"work in ESM-supporting browsers without build steps" isn't a universally agreed upon goal, nor one that's pleasant to support in node packages :-/ i'll step back and leave it to maintainers decide if that's something that they think is important.

ljharb avatar Apr 30 '20 18:04 ljharb

I can also step back to see what the maintainers say about the inclusion of a prebuilt ESM build for browsers, but I will point out:

  1. esquery already accepted a prebuilt ESM build for browsers.
  2. Another estools project, escodegen, already has a browser build, so these are apparently not intended as exclusively Node packages.

brettz9 avatar May 01 '20 01:05 brettz9

I went ahead and added the native ESM support with exports.

If this organization is dead the new releases, I would appreciate a heads up, as I've already started making forks of other projects, not having heard back. But I'm happy to submit my test improvements, etc. if someone still wishes to review and maintain.

brettz9 avatar Apr 07 '22 03:04 brettz9