search

whitlockjc / json-refs

Various utilities for JSON Pointers (http://tools.ietf.org/html/rfc6901) and JSON References (http://tools.ietf.org/html/draft-pbryan-zyp-json-ref-03).
MIT License
223 stars 63 forks source link

A few issues noticed as a result of a gulp attempt #200

Open brettz9 opened 1 year ago

brettz9 commented 1 year ago

Running the gulp script was giving the error:

ReferenceError: primordials is not defined

...which as per https://stackoverflow.com/a/58394828/271577 can be fixed by adding an overrides.

There were also a few linting errors, also fixed.

The browser tests still don't run, however.

I do have some energy at the moment, so am looking to make another, updated PR for ESM support, but have a couple questions on that:

  1. Do you still want to use gulp? If so, it seems updating to the latest packages can be made to work except that Istanbul doesn't currently work with ESM, so we'd need to move to c8 (there is a gulp-mocha-c8 plugin I could try).
  2. I'm not sure how well JSDoc will work with the ESM changes
  3. Are you ok dropping the very low minimum Node version you have now in engines in package.json to the LTS versions of Node? That should let us also have the binary in ESM without transpilation.
whitlockjc commented 1 year ago

Responses to your question:

  1. I want to move away from Gulp, I don't think it's necessary anymore.
  2. How so?
  3. We could/should change to the oldest LTS

Overall, can you suggest why you want to move to ESM? I'm not up to speed on its adoption. I do think the work I'm doing to build for browsers is likely unnecessary. I've started a few new JavaScript projects and I don't use Gulp, nor do I do any sort of Browser-specific builds anymore. I'm kinda out of the loop as far as best practices but I'm all for migrating to the best practices.

brettz9 commented 1 year ago

Responses to your question:

  1. I want to move away from Gulp, I don't think it's necessary anymore.

Ok.

  1. How so?

There was an outstanding bug regarding discovery of functions around export. I plan to see whether I have to use a similar workaround to what you were using or whether we can just attach export.

  1. We could/should change to the oldest LTS

Overall, can you suggest why you want to move to ESM? I'm not up to speed on its adoption.

Because it is becoming the standard both for browser and server (Node, Deno, etc.), it is compelling to do so now.

A popular maintainer of Node packages, sindresorhus, has, for example, been moving packages to ESM-only even: https://medium.com/sindre-sorhus/get-ready-for-esm-aa53530b3f77 . Packages that wish to use such packages must be ESM.

Besides this reason, there is the fact that for browsers (and I believe Deno), the only modular approach available without transpilation or use of a framework is ESM.

I do think the work I'm doing to build for browsers is likely unnecessary.

I'm a fan of browser builds. To me, not providing one is like not providing binaries. It might not be strictly necessary, but it's really cumbersome for the average consumer not to have one.

This is especially a problem if one's build process would require the extra work of transpiling CommonJS into ESM (i.e., if sources are not already all ESM)—as is the case here. It requires the consumer to be aware of one's implementation details. Something the modularity of ESM is meant to avoid.

While some will argue that ESM is not needed in the browser, with the goal being to have a single bundle that avoids extra HTTP requests where it is necessary to wait for the results to find additional imports, for the purposes of simple (and commonly needed) repo demos, it can be easier and cleaner to take advantage of the modularity of ESM.

And again, even if the consumer does wish to transpile (e.g., being keen to build with the latest dependency versions), they ideally won't need to be bothered by precise details of the build process (such as needing to know to introduce Node resolution and CommonJS plugins to their Rollup routine if the dependencies aren't already all ESM).

whitlockjc commented 1 year ago

I defer to your judgement. I just want to make it as easy to build/release as possible using best of breed approach in doing so. As for consumption, same thing.

brettz9 commented 1 year ago

Unfortunately, after getting most things in order for the proposed ESM PR, it seems the Rollup ecosystem for polyfilling Node in the browser does not work out of the box as it does with Webpack. There are packages like https://github.com/ionic-team/rollup-plugin-node-polyfills but errors are still coming through and although I made some progress working around them, some of the issues seem insurmountable without forking the polyfills project.

I'd still recommend merging this PR, however.