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

✨ Alias default exports with named exports #667

Merged
merged 4 commits into from
Dec 13, 2021
Merged

Conversation

wwilsman
Copy link
Contributor

What is this?

Default exports are a handy convenience for ES modules. However, with dynamic imports and commonjs modules, the default export must always be destructured due to default being a reserved word.

// esm
import thing from './thing';
// commonjs
const { default: thing } = require('./thing');
// dynamic import
const { default: thing } = await import('./thing');
// re-export
export { default as thing } from './thing';

By exporting defaults as named exports and aliasing them to default exports, much cleaner code can be written:

// esm
import thing from './thing';
// commonjs
const { thing } = require('./thing');
// dynamic import
const { thing } = await import ('./thing');
// re-export
export { thing } from './thing';

Tangential changes

  • While adding a named @percy/env export, I felt compelled to rename the class to match the package name more accurately (PercyEnv).

  • Packages that rely on @percy/core frequently need to perform some sort of request and utilize request from @percy/client to do so. When we add support for Yarn 2+, these packages will need to specifically declare a direct dependency on @percy/client, which will then trigger extra unnecessary dependency notifications.

    This small inconvenience can be alleviated by re-exporting request through @percy/core which already has a direct dependency on @percy/client. While this PR adds the necessary export, imports throughout other packages will need to be updated when adding support for Yarn 2+.

Packages that rely on `@percy/core` frequently need to perform some sort of request and utlize the
request util from `@percy/client` to do so. When linting for extraneous dependencies, these packages
need to specifically declare a direct dependency on `@percy/client`, which can cause unnecessary
dependency updates. This small inconvinience can be alleviated by re-exporting the request util from
`@percy/client` via `@percy/core` utils.
@wwilsman wwilsman requested a review from Robdel12 December 13, 2021 23:09
@wwilsman wwilsman enabled auto-merge (squash) December 13, 2021 23:27
Copy link
Contributor

@Robdel12 Robdel12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🏁

@@ -6,4 +6,4 @@
; ;
/ \
_____________/_ __ \_____________
Times we have broken CI: 2
Times we have broken CI: 3
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💥

@wwilsman wwilsman merged commit 369ab32 into master Dec 13, 2021
@wwilsman wwilsman deleted the ww/export-improvements branch December 13, 2021 23:40
@wwilsman wwilsman added the ✨ enhancement New feature or request label Dec 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
✨ enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants