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

[RFC] Next Step for @swc/helpers #7157

Closed
magic-akari opened this issue Mar 29, 2023 · 6 comments · Fixed by #7182
Closed

[RFC] Next Step for @swc/helpers #7157

magic-akari opened this issue Mar 29, 2023 · 6 comments · Fixed by #7182
Milestone

Comments

@magic-akari
Copy link
Member

magic-akari commented Mar 29, 2023

Describe the feature

Motivation

Current issues with @swc/helpers:

  1. default export used in helpers
  2. Different import paths used in CJS and ESM

The use of default export incurs the interopRequireDefault overhead when transpiling to CJS.
We should use named export to avoid it.

Additionally, keeping the exported function name consistent with the filename can reduce conversion complexity and aid in debugging.

CJS and ESM should have different module formats, but this should not be explicitly done in @swc/core.
We can use a unified import path and rely on the exports field to automatically select the suitable module format for runtime/bundler.

Better Way to introduce @swc/helpers

1. use named export

This is a simple changes and also the reason for breaking changes.

2. unified import path

To ensure semantic clarity and ease of debugging, we have opted to use esm and cjs path instead of src and lib.
Taking _array_like_to_array as an example, after relocating the module, the directory structure is as follows.

.
├── _/_array_like_to_array
│   └── package.json
├── cjs
│   └── _array_like_to_array.cjs
├── esm
│   └── _array_like_to_array.js
└── package.json

There are a few details to note, such as utilizing .js as the file extension for esm
and declaring type: module in the package.json.
This prioritizes the use of esm modules and hints the bundler to use them as much as possible.

{
  "type": "module",
  "exports": {
    "./_/_array_like_to_array": {
      "import": "./esm/_array_like_to_array.js",
      "default": "./cjs/_array_like_to_array.cjs"
    }
  }
}

By adding the exports field to the package.json file, we can benefit from conditional exports and use a uniform import path during translation.

// esm
import { _array_like_to_array } from "@swc/helpers/_/_array_like_to_array";

_array_like_to_array(foo);
// cjs
var _array_like_to_array = require("@swc/helpers/_/_array_like_to_array");

_array_like_to_array._array_like_to_array(foo);

note: I have chosen the path _ here to ensure the shortest possible pathway.

To ensure compatibility with bundlers/runtimes that do not support conditional exports,
we add the file ./_/_array_like_to_array/package.json with the following content:

{
  "main": "../../cjs/_array_like_to_array.cjs",
  "module": "../../esm/_array_like_to_array.js"
}

Babel plugin or link to the feature description

No response

Additional context

These changes would require introducing breaking changes, meaning that @swc/helpers would need to be updated to version 0.5.0.


I have published a @magic-akari/swc-helpers-experiments package. Compatibility testing is currently in progress and I will update the test results here.

@kdy1
Copy link
Member

kdy1 commented Mar 29, 2023

I think this is a good direction overall, and IMHO, a breaking change of @swc/helpers is inevitable given that we are doing something bad (default export) already.

@magic-akari
Copy link
Member Author

magic-akari commented Mar 29, 2023

I think this is a good direction overall, and IMHO, a breaking change of @swc/helpers is inevitable given that we are doing something bad (default export) already.

With this RFC design, the paths of src and lib are not used.
I believe compatibility can still be achieved through re-export.

// src/_array_like_to_array.mjs
export { _array_like_to_array as default } from "../esm/_array_like_to_array.js";
// lib/_array_like_to_array.js
module.exports = {
    get default() {
        return require("../cjs/_array_like_to_array.cjs")._array_like_to_array;
    }
}

Note:
❌ new version @swc/core with old version @swc/helpers
Since we will change to unified import path

⭕ old version @swc/core with new version @swc/helpers
reexport everything from esm and cjs


But, is it worthwhile to do so?

I cannot find the reason for upgrading @swc/core without upgrading @swc/helpers.

@kdy1
Copy link
Member

kdy1 commented Mar 29, 2023

I cannot find the reason for upgrading @swc/core without upgrading @swc/helpers.

It makes sense. Then I think it's fine anyway.

@magic-akari
Copy link
Member Author

magic-akari commented Mar 29, 2023

Compatibility result

esm cjs
node 12.16.0 🟡 🟢
node 12.17.0 🟢 🟢
node 12.22.12 🟢 🟢
node 14.21.3 🟢 🟢
node 16.19.1 🟢 🟢
node 18.15.0 🟢 🟢
webpack 4.46.0 🟢 🟡
webpack 5.76.3 🟢 🟢
rollup 3.20.2 🟢 🟡

Note:

  1. Node v12.16.0 is the last version which requires --experimental-modules flag.
  2. webpack@4 will load esm dependencies even in cjs environments. However, there is no need for concern. webpack@4 will handle the esm interoperability.
  3. rollup does not bundle dependencies from node_modules. This is by design. With @rollup/plugin-node-resolve, dependencies will be bundled in esm environments.

https://gist.github.com/a5e9619b8d797d2736bc08b6d74dae31.git

@magic-akari
Copy link
Member Author

Guidelines from webpack

  • Avoid the default export. It's handled differently between tooling. Only use named exports.
  • Never provide different APIs or semantics for different conditions.
  • Write your source code as ESM and transpile to CJS via babel, typescript or similar tools.
  • Either use .cjs or type: "commonjs" in package.json to clearly mark source code as CommonJs. This makes it statically detectable for tools if CommonJs or ESM is used. This is important for tools that only support ESM and no CommonJs.
  • ESM used in packages support the following types of requests:
    • module requests are supported, pointing to other packages with a package.json.
    • relative requests are supported, pointing to other files within the package.
      • They must not point to files outside of the package.
    • data: url requests are supported.
    • other absolute or server-relative requests are not supported by default, but they might be supported by some tools or environments.

https://webpack.js.org/guides/package-exports/#guidelines

@kdy1 kdy1 added this to the Planned milestone Apr 2, 2023
@kdy1 kdy1 closed this as completed in #7182 Apr 4, 2023
kdy1 pushed a commit that referenced this issue Apr 4, 2023
**BREAKING CHANGE:**

Breaking changes for `@swc/helpers`. A new major version `0.5.0` is required.


**Related issue:**

 - Closes #7157
@kdy1 kdy1 modified the milestones: Planned, v1.3.45 Apr 4, 2023
@swc-bot
Copy link
Collaborator

swc-bot commented May 4, 2023

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.

@swc-project swc-project locked as resolved and limited conversation to collaborators May 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging a pull request may close this issue.

3 participants