search

Chevrotain / chevrotain

Parser Building Toolkit for JavaScript
https://chevrotain.io
Apache License 2.0
2.51k stars 206 forks source link

Provide an ES6 build #976

Closed Mugen87 closed 3 years ago

Mugen87 commented 5 years ago

Status Update

Original Issue by @mugen87 below:

We are using chevrotain for our new VRMLLoader in three.js (see https://github.com/mrdoob/three.js/pull/16726). Since we currently migrate our official examples to an ES6 module code base, it would be great if all external libraries provide a respective module file, too. In this particular case, something like chevrotain.module.js 😊

bd82 commented 5 years ago

Is this something like: https://github.com/vuejs/vue/blob/dev/package.json#L6 ?

Do you have some references to explain why a UMD export is insufficient ? Is this scenario related to using ES6 modules directly in the browser without a transplantation phase?

Regarding the VRMLoader:

bd82 commented 5 years ago

Is this another example of what you desire? https://www.npmjs.com/package/lodash-es

Mugen87 commented 5 years ago

Is this something like: https://github.com/vuejs/vue/blob/dev/package.json#L6 ?

Yes, it seems Vue does it similar to three.js, see here. It's just a different naming convention of the file.

Is this scenario related to using ES6 modules directly in the browser without a transplantation phase?

Correct. But you can also use the modules when developing in an node environment and perform your build with e.g. rollup.

Is this just a three.js example or a full component bundled / dependency of three.js?

It's an independent component of three.js and no part of the main build. When you download the npm package of three.js, your imports look like so:

import { PerspectiveCamera, Scene, WebGLRenderer } from 'three';
import { VRMLLoader } from 'three/examples/jsm/loaders/VRMLLoader.js';
Mugen87 commented 5 years ago

Is this another example of what you desire? https://www.npmjs.com/package/lodash-es

It looks like every file of lodash is provided as ES6 modules. We are looking for a single file that exports chevrotain similar to Vue here:

https://github.com/vuejs/vue/blob/dev/dist/vue.esm.js#L11972

Then we could do:

import chevrotain from "module-name";

You could also consider to enable named imports but the way Vue works would be fine for us.

bd82 commented 5 years ago

I am looking for more references how popular libraries do esm bundling:

EqualMa commented 5 years ago

I am wondering if all the browsers that support module also support es6, then we don’t need es5+esm+bundle. So I searched for compatibility data of browsers but I don’t know how to find out

https://caniuse.com/#feat=es6-module https://caniuse.com/#feat=es6

bd82 commented 5 years ago

I am wondering if all the browsers that support module also support es6, then we don’t need es5+esm+bundle. So I searched for compatibility data of browsers but I don’t know how to find out

I am not sure about the bundled build requirement, but the ES5 requirement is more than just about browsers, For example somebody could use the "module" build to perform tree shacking for some application meant to run on node.js 8. Should we then limit the ES6 features we are using to those available in node.js 8.0 only?

It is easiest to target ES5 to ensure comparability instead of trying to prove 100% comparability in our usage of ES6 features. particularly considering both the target runtimes (browsers/nodejs) and the used compiler (tsc) are changing variables...

EqualMa commented 5 years ago

To summarize this issue:

To provide a build, we should pick from the followings:

  1. es6 or es5
  2. esm or umd or commonjs
  3. bundle (and minified bundle) or not

Now we have es5+commonjs, es5+umd+bundle, we are discussing about the following builds: es6+esm, es6+esm+bundle, es5+esm, es5+esm+bundle

Should we provide all the above builds or just some of them? If we choose some builds, how do we choose?

bd82 commented 5 years ago

Should we provide all the above builds or just some of them? If we choose some builds, how do we choose?

I am thinking about targeting lowest common denominator here.

Reasons:

Mugen87 commented 5 years ago

Well, a bundle like chevrotain.module.js would still be handy for use (and maybe other projects) and it should be easy to configure this with rollup 😇

bd82 commented 5 years ago

Well, a bundle like chevrotain.module.js would still be handy for use (and maybe other projects) and it should be easy to configure this with rollup.

I am still slowly in the process of understanding this topic so forgive me if I ask any "stupid questions" 😸

I am not generally opposed to additional artifacts for ease of consumption. But I am worried about the overhead:

So perhaps if it is:

Then we do not need to take deal with an esm bundle at this time and limit the scope to un-bundled esm artifacts.

Mugen87 commented 5 years ago

Sry, I was not aware that you are not using rollup. In this case it might be more complicated to provide such a build. Unfortunately, I'm not familiar with webpack so I'm not a big help in this context.

Devs do not always use npm to install dependencies or want to configure a build. I think it makes a library easier to use if it is accessible over UMD or ESM bundle files. That's the reason why I would love to see chevrotain.module.js^^.

bd82 commented 5 years ago

Devs do not always use npm to install dependencies or want to configure a build. I think it makes a library easier to use if it is accessible over UMD or ESM bundle files. That's the reason why I would love to see chevrotain.module.js^^.

I agree that is why Chevrotain is already providing a UMD build (both minified and unminified).

I think the first step would be to provide an esm + es5 + un-bundled artifact. which can be accomplished without any new tooling (by using TSC).

The second step would be to investigate farther how common it is to also provide bundled esm modules in the JavaScript eco-system and if it considered a good/best practice try to implement that as well.

I think we can run the tests for the esm bundles using --experimental-modules CLI argument on modern version of node.js.

StoneCypher commented 4 years ago

At this point it is common to provide only es6 modules, and expect downstreams to use a transcompiler or packager if they want es5

In practice, being tied to the past causes more problems than supporting it

The ostensible expected behavior is to write modern es6, then compile it down to es5 using something like typescript, babel, buble, et cetera; to expose the es6 on the package.json module, and to expose the es5 on package.json main

Rollup can produce a minified single file es5 build for you from modern es6, or any of the other described formats here

bd82 commented 4 years ago

The ostensible expected behavior is to write modern es6, then compile it down to es5 using something like typescript, babel, buble, et cetera; to expose the es6 on the package.json module, and to expose the es5 on package.json main

That is what is happening now: https://github.com/SAP/chevrotain/blob/master/packages/chevrotain/package.json#L36-L37

The open question left is if to provide a minified ES6 build too or not.

StoneCypher commented 4 years ago

Oh. Unless you're compile time sensitive (like if this is being done on a developer's machine instead of by CI,) yes, you should.

The shims to support ES6-in-ES5 are frequently quite heavy. If you're offering a minified build, it's to people who are binary space sensitive. Minified es5-of-es6 is often ~15% or more larger than minified-es6, using the same minifier. The shims don't go away.

If someone's using a min and has es6 available, they'll get a decent sized boost out of this.

You'll get another size boon out of using a non-framing bundler (like rollup) instead of a framing bundler (like webpack.)

StoneCypher commented 4 years ago

There are some things I don't understand about this build process.

  1. You're using typescript. What's esm for?
  2. It looks like you have parallel test processes - karma wrapping mocha for es5, and mocha for es6. Am I misreading that?
    1. Is that to test that both the es5 and es6 packaging work as expected?
    2. Do you also intend to test the minified packages?
bd82 commented 4 years ago

If I understand you correctly and also consider that ES6 sources are more likely to be easily go through tree shaking that providing an un-minified ES6 build is sufficient and maybe even optimal (size wise) for anyone who wants to create their own minified ES6 bundle?

bd82 commented 4 years ago

You're using typescript. What's esm for?

There are two builds of TypeScript, one using ES6 modules and one targeting ES5 for legacy.

It looks like you have parallel test processes - karma wrapping mocha for es5, and mocha for es6. Am I misreading that? Is that to test that both the es5 and es6 packaging work as expected? Do you also intend to test the minified packages?

I think there may be a slight misunderstanding:

The important thing to note is that all artifacts are tested in as many envs as possible. And that includes minified bundles.

Maybe I should consider removing IE11 and ES5.1 support and only exporting ES6 modules. But I get the feeling this will have to wait for all Active Node versions to support ESM first...

StoneCypher commented 4 years ago

If I understand you correctly and also consider that ES6 sources are more likely to be easily go through tree shaking that providing an un-minified ES6 build is sufficient and maybe even optimal (size wise) for anyone who wants to create their own minified ES6 bundle?

Tree shaking is also an interest, but in the real world, most es5 is tree-shakeable these days. Granted, es6 tree shaking is somewhat better, but not by that much.

But no. I'm saying "the transcompiled code is larger than the original code, to a sigificant degree even after minification."

I'd much rather have the minified left one than the minified right one.

image

The important thing to note is that all artifacts are tested in as many envs as possible. And that includes minified bundles.

Ah. Surprised you aren't using a GH actions matrix to do browser/OS/build then

I can show you how if you're interested

Going to sleep for now, it's 1:30a

bd82 commented 4 years ago

But no. I'm saying "the transcompiled code is larger than the original code, to a sigificant degree even after minification."

I'd much rather have the minified left one than the minified right one.

The ES6 un-minified files are part of the package

So you can already build those more efficient minified files.

bd82 commented 4 years ago

Ah. Surprised you aren't using a GH actions matrix to do browser/OS/build then

I used Travis in the past, now using Circle-CI and am very happy with it.

In the long term I want to simplify the project's dev-ops from TCO/Maintenance perspective, e.g removing all browser testing and targeting ES6 only (maybe in 2022? once Node.js 12 is deprecated).

StoneCypher commented 4 years ago

So you can already build those more efficient minified files.

@bd82 - Either way, doesn't much matter to me. But if you observe that unminified es6 is present, why not also observe that unminified es5 is present?

The reason for pre-building min files is threefold.

  1. Downstream users are much less capable than most people expect
  2. Lots of times you need to pick an asset from a node_module and compiling it is severely problematic
  3. CDNs are only going to offer some existing asset (this is the big one)

So like, if you want on CDNjs, their rules require you to have your mins already ready to go. If you also want a downstream user to be able to tree-shake at that point, the min needs to be in the repo, ostensibly in a dist directory.

Mind you, I don't actually care very much personally. I don't bother to minify my own libraries

It's just odd to do it for es5 then not for es6

.

What would be the advantage of GH Actions versus Circle-CI?

Three things.

  1. Matrix builds (this is the big one)
  2. Community familiarity
  3. Action sequences

2 first. Look, if I set my library up in Phabricator, or taiga.io, or gitea , ... there's some really powerful stuff in those systems. But also I'm never getting help with the build again

Because literally nobody in human history has ever understood phabricator. Not even its author. So if I screw something up in the build, or if there's an opportunity, it's on me to figure it out

Github actions, that's a common setup. Very soon everybody will be able to pitch in. Right now, Circle also kind of has this, but that's disappearing, fast.

3 next. GH actions are modular and invokeable from repos. That means that other people can create them. So, by example, I used to have typedoc in my projects; now, I've removed it, and I run the typedoc action instead. It was startling to me how much my projects shrank when I started externalizing the build tools. But that's not a big deal in this context.

What is is #1. I'll just show you one of my configs. Read it. I know, it's boring. It's worth it.

name: GH Actions example for bd82

on: [push]

jobs:
  build:

    strategy:
      matrix:
        node-version: [12.x, 13.x]
        browser: ['firefox', 'chromium']
        os: ['windows-latest', 'macos-latest', 'ubuntu-latest']

    runs-on: ${{ matrix.os }}

    steps:
    - uses: actions/checkout@v2
    - name: Use Node.js ${{ matrix.node-version }}
      uses: actions/setup-node@v1
      with:
        node-version: ${{ matrix.node-version }}
    - run: npm install
    - run: npm run build
    - run: npm test
      env:
        BROWSER: ${{ matrix.browser }}
    - name: export filename
      id: getfilename
      run: echo "::set-output name=file::$(ls screens/*.png)"
    - name: upload screen ${{ steps.getfilename.outputs.file }}
      uses: actions/upload-artifact@v1.0.0
      with:
        name: screenshot
        path: ${{ steps.getfilename.outputs.file }}

And then here's the driver from Javascript land:

const os         = require('os'),
      playwright = require('playwright');

const uBrowser  = process.env.BROWSER,
      supported = ['chromium', 'firefox', 'webkit'];

const osString = `${os.platform}-${os.arch}`,
      nodeVer  = `${process.version}`,
      path     = `./screens/screen-${uBrowser}-${osString}-node${nodeVer}.png`;

(async () => {

  if (supported.includes(uBrowser)) {

    const browser = await playwright[uBrowser].launch(),
          context = await browser.newContext();
          page    = await context.newPage('http://whatsmyuseragent.org/');

    await page.screenshot({ path });
    await browser.close();

  } else {
    throw `Unknown browser requested: ${uBrowser}`;
  }

})();

Finally, put that in a node project that also has typescript, express, and playwright, and put this as the npm run test script block:

 "test": "mkdir screens && node src/js/instance.js"

Pow. You're done.

Commit that. Github Actions will spawn 18 builds, in 18 parallel VMs, for you. It will run ten of them at any time, so you'll have around the runtime of two individual builds.

You will see every combination of:

.

In the long term I want to simplify the project's dev-ops from TCO/Maintenance perspective, e.g removing all browser testing and targeting ES6 only (maybe in 2022? once Node.js 12 is deprecated).

If the goal is to simplify devops, no need, gh actions + playwright can do the job with a few-line script that's durable basically forever.

If the goal is to simplify test development/maintenance, I think this is probably a smart choice.

StoneCypher commented 4 years ago

That thing is even collecting screenshots from every browser at the end of the test.

If you look at the javascript driver, what it's actually taking screenshots of is the webpage http://whatsmyuseragent.com/`, to show that it really is a fleet of browsers, on a fleet of operating systems, doing the check

This driver makes no effort to involve the node version in the screenshot because I got bored

StoneCypher commented 4 years ago

Also, playwright knows how to imitate safari, including safari mobile, as well as android chrome

This won't help with exotics like opera, samsung internet, pre-chromium Internet Explorer, edge, et cetera. But honestly who cares

bd82 commented 4 years ago

Minification

The reason for pre-building min files is threefold.

  1. Downstream users are much less capable than most people expect
  2. Lots of times you need to pick an asset from a node_module and compiling it is severely problematic
  3. CDNs are only going to offer some existing asset (this is the big one)

Hmmm #3 is indeed the big one, but perhaps with HTTP 2.0 and ES6 that would become less important.

I am indeed debating on the inclusion of minified ES6 bundle, on one hand as you said it would align with the minified ES6 bundle, on the other hand I would not want to provide any minified bundle at all...

Github Actions.

Thanks for the examples. Knowing the GH Actions has a matrix support may be handy in the future. Anyhow I strongly prefer to avoid extracting the build logic out of the repository (e.g your TypeDoc example). As I want to be able to reproduce the build locally as much as possible and I want the build to be easily portable between different systems (e.g switching between travis->circle-ci).

There is also the concern of coupling to GitHub Specific APIs, e.g what if a repo wants to move to bitbucket or gitlabs?

Browser Testing.

Thanks for bringing playwright to my attention, I used to run karma tests with SauceLabs on multiple browsers (including IE11/Safari) but that was sometimes not 100% stable...

At this point I'm leaning towards removing the browser tests completely, particularly as this library is primarily used in node.js...

Summary

Thanks for the discussion and information 👍 It may not all be relevant to this repo, but the information will come in handy in other projects...

In this specific project I am currently more interested in simplification:

So some of these suggestions may be less suitable...

Cheers. Shahar.

StoneCypher commented 4 years ago

To be clear: I am not pushing for my ideas. I'm just following through because to me it's an interesting discussion.

.

The es6/http2 "we don't need to minify anymore" thing is in my opinion probably wrong.

The thing is that I've tried it already, and the browser takes longer to parse the local JS and figure out what to request next than the far end would have needed to finish shipping

It just introduces local JS processing as a network delay stage

.

Knowing the GH Actions has a matrix support may be handy in the future.

It has been an absolute life-changer for me.

.

As I want to be able to reproduce the build locally as much as possible and I want the build to be easily portable between different systems (e.g switching between travis->circle-ci).

This is a valid consideration, and I used to see things this way too.

The thing is, today things aren't switchable between travis and circle, and circle is literally travis' codebase used a second time. For that matter, projects also generally can't be taken between two instances of Jenkins, even the same version. I don't think this kind of portability exists in the real world.

So then to me, the question becomes "what is the actual work of porting"

Well, it was moving things from this package json to that one over there

Literally a single copy paste 😄

To me, it seems like putting the JS that does build steps in a separate directory from the JS that's actually the library. nothing's changed, it's just in a different location so you don't have to think about it at the same time.

But also, just do what works for you 😀

.

There is also the concern of coupling to GitHub Specific APIs, e.g what if a repo wants to move to bitbucket or gitlabs?

I have never seen this go well, frankly

GitLab is behind, and BitBucket belongs in the 1990s

I think it's a little like saying "what if I want to use PHP, or Phabricator?"

The short answer is "that won't be happening"

.

At this point I'm leaning towards removing the browser tests completely, particularly as this library is primarily used in node.js...

Up to you

Still, evergreen browser testing, which was a nightmare under karma, is now pretty straightforward.

.

Splitting the project into multiple smaller packages.

Peg made this mistake. Contributions basically disappeared.

It's way too confusing looking across multiple packages for the thing you're trying to fix

bd82 commented 4 years ago

To be clear: I am not pushing for my ideas. I'm just following through because to me it's an interesting discussion

No worries 😄 I am obtaining valuable input from this discussion its all good...

It just introduces local JS processing as a network delay stage

Good point.

The thing is, today things aren't switchable between travis and circle, and circle is literally travis' codebase used a second time. For that matter, projects also generally can't be taken between two instances of Jenkins, even the same version. I don't think this kind of portability exists in the real world.

Hmm I have switched from Travis to Circle and from Azure-Pipelines to Circle-CI pretty easily. It is not 100% portable, however if you ensure your project devDeps are well encapsulated and avoid dependencies to other runtime tools (e.g Python CLIs). it can be 97.5% portable.

GitLab is behind, and BitBucket belongs in the 1990s I think it's a little like saying "what if I want to use PHP, or Phabricator?" The short answer is "that won't be happening"

I agree it would likely not happen anytime soon, but what about 5 years down the road? who knows...

Still, evergreen browser testing, which was a nightmare under karma, is now pretty straightforward.

I will keep this in mind when I have a project where the evergreen browser testing is indeed important, thanks 👍

Peg made this mistake. Contributions basically disappeared. It's way too confusing looking across multiple packages for the thing you're trying to fix.

There are many mono-repos with multiple packages and large active communities. Also consider that Chevrotain's productive code base is approx twice as large as PegJS. I think it may even make things more accessible, as it could be a-lot less scary to dive into a 1,000 LoC package than a 10,000 LoC package...

StoneCypher commented 4 years ago

Hmm I have switched from Travis to Circle and from Azure-Pipelines to Circle-CI pretty easily.

Azure Pipelines is the same thing as github actions

If you look at your actions tab you will see any azure pipelines builds from the last six months

It is not 100% portable, however if you ensure your project devDeps are well encapsulated and avoid dependencies to other runtime tools (e.g Python CLIs). it can be 97.5% portable.

This was also my point

.

I agree it would likely not happen anytime soon, but what about 5 years down the road? who knows...

I would be willing to gamble against it at virtually any price

.

There are many mono-repos with multiple packages and large active communities

Honesty not really

If you look at the more obvious cases like react, there's always a large durable loss in commits

If you look at the smaller cases it's /usually/ fatal

bd82 commented 3 years ago

closing this, will be tracked in: #1383