Skip to content
Permalink

Comparing changes

Choose two branches to see what’s changed or to start a new pull request. If you need to, you can also or learn more about diff comparisons.

Open a pull request

Create a new pull request by comparing changes across two branches. If you need to, you can also . Learn more about diff comparisons here.
base repository: janl/mustache.js
Failed to load repositories. Confirm that selected base ref is valid, then try again.
Loading
base: master
Choose a base ref
...
head repository: janl/mustache.js
Failed to load repositories. Confirm that selected head ref is valid, then try again.
Loading
compare: remove-build-output-from-git
Choose a head ref
  • 7 commits
  • 15 files changed
  • 1 contributor

Commits on Feb 22, 2021

  1. Rename .mjs -> .js to make it ESM and not have build output in git 

    Although we'll now end up with only `mustache.js` in the git repo,
being written in ESM syntax, the npm package will still be as before:

- `mustache.js`: UMD, meaning CommonJS, AMD or global scope
- `mustache.mjs`: ESM
    @phillipj
    phillipj committed Feb 22, 2021
    Configuration menu
    Copy the full SHA
    81c4b29 View commit details
    Browse the repository at this point in the history

  2. Remove .min.js from git repository, will be kept in npm package

    @phillipj
    phillipj committed Feb 22, 2021
    Configuration menu
    Copy the full SHA
    a10fba2 View commit details
    Browse the repository at this point in the history

  3. Bump mustache.js version via npm script instead of git pre-commit hook 

    Primarily because installing `git` hooks are easy to forget for
maintainers, whilst telling `npm` to do something as a consequence of
`$ npm version <patch | minor | major>` is seamless.

Also worth mentioning we what we used to do in the pre-commit hook
script is now greatly simplified as we don't want to have the build
output and/or the `.min.js` in git anymore.

Therefore, as long as we've bumped the version number in the source
code, we're basically done, after having amended that change into the
git commit the `npm` CLI creates.
    @phillipj
    phillipj committed Feb 22, 2021
    Configuration menu
    Copy the full SHA
    156e4fb View commit details
    Browse the repository at this point in the history

  4. Use esm package locally when testing to use ESM syntax from CJS code 

    These changes are primarily aiming for having an acceptable developer
experience when working on changes locally and want to run the test
suite.

By using `esm` as a required package to be executed *before* the
individual test files are executed, `esm` allows CommonJS/`require()`
based code to use code written with ES Modules syntax.

In practise that means our existing CommonJS based test suite that is
`require()`ing the source code witten in ES Modules, is seamless and
totally transparent. It just works without any build or transpiling
pipeline like `babel` involved.

Caveat: the `esm` package only support Node.js 6.x and above.

That is more than okey, as we've got continous integration to verify
how our changes works on different versions of Node.js, browsers & deno.

Refs https://www.npmjs.com/package/esm
    @phillipj
    phillipj committed Feb 22, 2021
    Configuration menu
    Copy the full SHA
    126d6d8 View commit details
    Browse the repository at this point in the history

  5. Avoid use of esm when running tests on legacy Node.js versions 

    Because `esm` does not support legacy versions of Node.js, at least not
below Node.js 6 as of writing this, we have to avoid using that whenever
tests are running on legacy versions.

But now that the source code (`mustache.js`) is written in ESM syntax,
how on earth are we going to run tests on these legacy versions of
Node.js? We gotta run the build step first, so that we end up with a
`mustache.js` file in CJS, or strictly speaking it will be UMD.

That's kinda pain in the backside isn't it? Yes, but running tests on
legacy versions are not meant to be done locally, but rather in CI. That
means we can easily automate the flow of (1) building the source code
before (2) starting the test suite.

For our futureselves, if we want to stop running tests on legacy
versions of Node.js; the changes introduces in this commit, could be
removed completely.
    @phillipj
    phillipj committed Feb 22, 2021
    Configuration menu
    Copy the full SHA
    02c9ef1 View commit details
    Browse the repository at this point in the history

  6. Build ESM -> CJS before running legacy Node.js tests and packaging tests 

    Because the source code is written in ESM syntax and we cannot use the
`esm` package to make `require()` ESM compatible, since that package
does not support legacy versions of Node.js.

Also in our usage tests in CI, we need to mimck the npm packaging
behaviour where building is involved.
    @phillipj
    phillipj committed Feb 22, 2021
    Configuration menu
    Copy the full SHA
    e9fcdc1 View commit details
    Browse the repository at this point in the history

  7. Build ESM -> CJS before running tests in browsers via Saucelabs 

    Noteworthy tweak is the fact that it seems `zuul` are eagerly loading
all `require('whatever-package-or-path')` it sees before pushing the
contents to Saucelabs.

That was troublesome because we don't want the `esm` package to be
loaded when running browser tests. But it was, even though we loaded
`esm` conditionally after checking the current Node.js version used.

The trick was to not use `require('esm')` but `module.require('esm')`
instead, to fool the assumed look-out for `require` somewhere inside
`zuul`'s code.
    @phillipj
    phillipj committed Feb 22, 2021
    Configuration menu
    Copy the full SHA
    5090644 View commit details
    Browse the repository at this point in the history
Loading