Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

- Create as native ESM; refactor tests to ESM #55

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

brettz9
Copy link

@brettz9 brettz9 commented May 14, 2021

This is a breaking change in adding engines to 12 (though Node 10 is now EOL).

It is also breaking because now only the paths added to exports are available, but I imagine providing the UMD and ESM distribution files as well as TypeScript declaration file should be all that ought to be desirable.

Besides adding native ESM, it also adds an ESM distribution which browser might use directly (and without needing a TypeScript build step).

@brettz9
Copy link
Author

brettz9 commented May 15, 2021

Also wondering if you might have some idea if and when you might get to a review on these PRs? Thanks!

@brettz9
Copy link
Author

brettz9 commented May 29, 2021

Hi, any chance you might review this and the other PRs, @boblauer (gradually or as you prefer) ? Thanks!

@boblauer
Copy link
Owner

I'm not too familiar with native esm modules, so this is hard for me to review and fully understand the implications.

It is also breaking because now only the paths added to exports are available

Can you expand upon what this means?

@brettz9
Copy link
Author

brettz9 commented May 29, 2021

Sure. Node can now import and export ESM modules (as of Node 12 especially) just as it can export CommonJS-style module.exports or exports, without any need for bundling down to CommonJS.

However, since people will no doubt still be using CommonJS for some time, setting exports in package.json allows for specifying where the ESM distribution file can be found, and where the CommonJS distribution file can be found so that Node can auto-discover this.

By default, package encapsulation occurs when adding exports to package.json, meaning that modern Node versions will no longer allow Node users to import/require files within the package unless they are exposed on exports in package.json. So if someone previously was calling require('mockdate/some/nested/file.js'), and that file was not whitelisted on package.json's exports, new Node versions won't allow that file to be loaded anymore. So, it is generally considered a breaking change to add to exports for this reason (though in the case of Mockdate), I don't think you really have other files you'd want to expose).

In this PR, I've kept with this principle of encapsulation and only allowed access to :

  • "./lib/mockdate.d.ts" (for TypeScript consumption of the declaration file)
  • "./lib/mockdate.js" (for ESM imports)
  • "./lib/mockdate.cjs" (for old-style CommonJS imports)

I don't think you really have any other files that you'd want to expose, but if you do, I could add them.

@brettz9
Copy link
Author

brettz9 commented Jul 21, 2021

Any clarifying questions?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants