Add package.json `exports` field #773

manzt · 2021-03-03T20:55:56Z

Adds the new "exports" field in package.json to define conditional exports for mustache.js (ESM & CJS). This field is used both by Node and bundlers to resolve dependencies. Since there is no "module" entrypoint for this package, some bundlers (rollup + @rollup/plugin-node-resolve) cannot resolve the module export. This is a step towards making both modules discoverable.

Example

// package.json
{
  "main": "index.js",
  "type": "module",
  "scripts": {
    "build": "rollup -c"
  },
  "dependencies": {
    "@rollup/plugin-node-resolve": "^11.2.0",
    "mustache": "^4.1.0",
    "rollup": "^2.40.0"
  }
}

import Mustache from 'mustache';

const view = {
  title: "Joe",
  calc: function () {
    return 2 + 4;
  }
};

const output = Mustache.render("{{title}} spends {{calc}}", view);
console.log(output)

// rollup.config.js
import resolve from '@rollup/plugin-node-resolve';

export default {
  input: 'index.js',
  output: { file: 'bundle.js', format: 'esm' },
  plugins: [ resolve() ]
}

❯ npm run build

> rollup -c


index.js → bundle.js...
(!) `this` has been rewritten to `undefined`
https://rollupjs.org/guide/en/#error-this-is-undefined
node_modules/mustache/mustache.js
4:   typeof define === 'function' && define.amd ? define(factory) :
5:   (global = global || self, global.Mustache = factory());
6: }(this, (function () { 'use strict';
     ^
7:
8:   /*!
[!] Error: 'default' is not exported by node_modules/mustache/mustache.js, imported by index.js
https://rollupjs.org/guide/en/#error-name-is-not-exported-by-module
index.js (1:7)
1: import Mustache from 'mustache';
          ^
2:
3: const view = {
Error: 'default' is not exported by node_modules/mustache/mustache.js, imported by index.js
    at error (/Users/fm813/github/manzt/reference-spec-reader/node_modules/rollup/dist/shared/rollup.js:5279:30)
    at Module.error (/Users/fm813/github/manzt/reference-spec-reader/node_modules/rollup/dist/shared/rollup.js:9996:16)
    at Module.traceVariable (/Users/fm813/github/manzt/reference-spec-reader/node_modules/rollup/dist/shared/rollup.js:10389:29)
    at ModuleScope.findVariable (/Users/fm813/github/manzt/reference-spec-reader/node_modules/rollup/dist/shared/rollup.js:8847:39)
    at MemberExpression.bind (/Users/fm813/github/manzt/reference-spec-reader/node_modules/rollup/dist/shared/rollup.js:6535:49)
    at CallExpression.bind (/Users/fm813/github/manzt/reference-spec-reader/node_modules/rollup/dist/shared/rollup.js:2728:23)
    at CallExpression.bind (/Users/fm813/github/manzt/reference-spec-reader/node_modules/rollup/dist/shared/rollup.js:6730:15)
    at VariableDeclarator.bind (/Users/fm813/github/manzt/reference-spec-reader/node_modules/rollup/dist/shared/rollup.js:2728:23)
    at VariableDeclaration.bind (/Users/fm813/github/manzt/reference-spec-reader/node_modules/rollup/dist/shared/rollup.js:2724:31)
    at Program.bind (/Users/fm813/github/manzt/reference-spec-reader/node_modules/rollup/dist/shared/rollup.js:2724:31)

npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! @ build: `rollup -c`
npm ERR! Exit status 1
npm ERR!
npm ERR! Failed at the @ build script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /Users/fm813/.npm/_logs/2021-03-03T20_51_56_547Z-debug.log

phillipj · 2021-03-03T21:20:02Z

Thanks @manzt!

Getting an exports field in, is indeed interesting.

Heads up about a few changes planned to be merge from #772, which touches upon the GitHub usage tests at least..

manzt · 2021-03-03T21:38:26Z

No problem. Let me know if there is something else I can do to help. I notice skypack is mentioned in #772; FYI importing mustache currently via skypack returns a (skypack generated) ESM wrapping around the CJS module because skypack can't find ESM source (it uses package exports).

<script type="module">
  import mustache from 'https://cdn.skypack.dev/mustache'; // skypack resolves mustache.js -> transpiles to ESM -> sends back
</script>

skypack cdn source: https://cdn.skypack.dev/-/mustache@v4.1.0-SsFnz9aXKP0TK1yt4fOG/dist=es2020,mode=imports/optimized/mustache.js

EDIT: Also, I totally sympathize with trying to target multiple environments. This project does so much more than most to make usage seamless. Thanks so much, it's a great library.

phillipj · 2021-03-05T20:38:14Z

Myeah, I also noticed the skypack story isn't ideal ATM. Guess it works, but let's try and do better 😄

Now that #772 has been merged, I'll try to wrap my head around the changes in this PR. I'm also pretty sure I've seen mentioned that adding an exports field should be seen as a semver major change. Does that ring a bell to you? I'll try and track down where I got those rumours from..

This project does so much more than most to make usage seamless. Thanks so much, it's a great library.

Really appreciate hearing that, thanks a bunch!

manzt · 2021-03-09T15:12:20Z

I'm also pretty sure I've seen mentioned that adding an exports field should be seen as a semver major change. Does that ring a bell to you?

I had not seen this before, but I'm glad that you had! Feeling a bit ignorant for suggesting such a major change when this is clearly stated in the Node.js docs: https://nodejs.org/api/packages.html#packages_package_entry_points

Warning: Introducing the "exports" field prevents consumers of a package from using any entry points that are not defined, including the package.json (e.g. require('your-package/package.json')). This will likely be a breaking change.

This change is apparently to allow library authors to encapsulate their packages, allowing fine-grain control over public interfaces for packages. With that said, I noticed the previous ESM imports in node were mustache/mustache.mjs so I added the export "./*": "./*" naively to keep previous tests passing. This is also mentioned in the docs:

As a last resort, package encapsulation can be disabled entirely by creating an export for the root of the package "./*": "./*". This exposes every file in the package at the cost of disabling the encapsulation and potential tooling benefits this provides.

TL;DR I think the "./*": "./*" export negates requiring a semver major change, but it's recommended as a "last resort" because you lose the encapsulation that export maps offer to library authors. If the latter is important, it probably makes sense to wait for a semver major change.

phillipj · 2021-03-14T09:37:15Z

Thanks @manzt, great research 👍🏻

With that said, I noticed the previous ESM imports in node were mustache/mustache.mjs so I added the export "./*": "./*" naively to keep previous tests passing

Sounds good to me, as that also ensures we keep backwards compatibility with what we've been telling ESM users to do for well over a year.

I think the "./*": "./*" export negates requiring a semver major change, but it's recommended as a "last resort" because you lose the encapsulation that export maps offer to library authors.

I don't consider encapsulation via the exports field important for us to be honest. The majority of what the npm package consists of, is only one .js file after all.

Looking at what our resulting package tarball consists of:

npm notice === Tarball Contents === 
npm notice 1.2kB  LICENSE                           
npm notice 3.5kB  bin/mustache                      
npm notice 25.2kB mustache.js                       
npm notice 11.8kB mustache.min.js                   
npm notice 2.0kB  package.json                      
npm notice 22.1kB CHANGELOG.md                      
npm notice 16.8kB README.md                         
npm notice 23.5kB mustache.mjs                      
npm notice 58B    wrappers/dojo/mustache.js.post    
npm notice 359B   wrappers/jquery/mustache.js.post  
npm notice 128B   wrappers/mootools/mustache.js.post
npm notice 184B   wrappers/qooxdoo/mustache.js.post 
npm notice 45B    wrappers/yui3/mustache.js.post    
npm notice 192B   wrappers/dojo/mustache.js.pre     
npm notice 141B   wrappers/jquery/mustache.js.pre   
npm notice 14B    wrappers/mootools/mustache.js.pre 
npm notice 6.4kB  wrappers/qooxdoo/mustache.js.pre  
npm notice 34B    wrappers/yui3/mustache.js.pre

..I don't see any of those files as "private" implementation details we'd want to avoid exposing to the outside. That's basically what we've been using the files field for already:

mustache.js/package.json

Lines 21 to 26 in 3e29d67

    
           "files": [ 
        
             "bin/", 
        
             "mustache.mjs", 
        
             "mustache.min.js", 
        
             "wrappers/" 
        
           ],

Bottom line: I think this is good to go.

phillipj · 2021-03-14T10:13:20Z

Thanks a lot for making this happen, I'm sure it'll make many devs out there happy 💯

I published 4.2.0-beta.0 to npm if anyone would like to give this a shot ASAP:

$ npm install mustache@beta

phillipj · 2021-03-28T19:32:45Z

This has now been elevated from beta -> latest and pushed to npm as v4.2.0.

manzt added 3 commits March 3, 2021 15:32

Add package exports

e80ea47

Add wildcard for submodules; fix existing esm test and add new.

da78692

add github action

950bfc6

phillipj changed the title ~~Add package exports~~ Add package.json exports field Mar 14, 2021

phillipj merged commit ea3adcf into janl:master Mar 14, 2021

manzt deleted the add-package-exports branch March 14, 2021 15:56

phillipj mentioned this pull request Jun 29, 2021

4.2.0 broken (typescript + webpack): #786

Open

alaingiller mentioned this pull request Jul 16, 2021

Mustache 4.2.0 is not correctly imported vercel/ncc#730

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add package.json `exports` field #773

Add package.json `exports` field #773

manzt commented Mar 3, 2021

phillipj commented Mar 3, 2021

manzt commented Mar 3, 2021 •

edited

Loading

phillipj commented Mar 5, 2021

manzt commented Mar 9, 2021

phillipj commented Mar 14, 2021 •

edited

Loading

phillipj commented Mar 14, 2021

phillipj commented Mar 28, 2021

Add package.json exports field #773

Add package.json exports field #773

Conversation

manzt commented Mar 3, 2021

Example

phillipj commented Mar 3, 2021

manzt commented Mar 3, 2021 • edited Loading

phillipj commented Mar 5, 2021

manzt commented Mar 9, 2021

phillipj commented Mar 14, 2021 • edited Loading

phillipj commented Mar 14, 2021

phillipj commented Mar 28, 2021

Add package.json `exports` field #773

Add package.json `exports` field #773

manzt commented Mar 3, 2021 •

edited

Loading

phillipj commented Mar 14, 2021 •

edited

Loading