feat(core): add AMD output format support

[bhovhannes]

↪️ Pull Request

This PR adds a new outputFormat - amd, which. will create bundle in AMD format.

This is related to #7312, and most probably code in this MR will be slightly refactored later to reuse as much as it is possible for UMD support.

But in any case this is a separate feature, and deserves a separate MR.

💻 Examples

Essentially, I want this to work:

{
  "source": "src/index.js",
  "main": "dist/index.js",
  "targets": {
    "main": {
      "context": "browser",
      "outputFormat": "amd",
      "includeNodeModules": {
        "lodash": false,
        "react": false,
        "react-dom": false
      }
    }
  }
}

🚨 Test instructions

✔️ PR Todo

• Added/updated unit tests for this change
• Filled out test instructions (In case there aren't any unit tests)
• Included links to related issues/PRs

[height]

Link Height tasks by mentioning a task ID in the pull request title or commit messages, or description and comments with the keyword link (e.g. "Link T-123").

💡Tip: You can also use "Close T-X" to automatically close a task when the pull request is merged.

[bhovhannes]

In this MR dynamic imports are not supported. I am still working on handling them properly.

[bhovhannes]

Being new to Parcel codebase I have a couple questions:

1. Which type of tests are preferred? I've identified about 10 possible scenarios I want to cover with tests, and I've noticed packages/core/integration-tests/test/output-formats.js, but I am not sure if it is desirable to increase number of integration tests that much (because they are slow). I didn't find any unit tests for ScopeHoistingPackager.js, which confuses me.

2. Should we use modern ES6 syntax in generated code to save a few bytes for supported environments or we can write es5-compatible code in all cases to make Parcel code shorter? Asking because I saw this.packager.bundle.env.supports('arrow-functions', true) condition in GlobalOutputFormat.js but without that conditions the code of AMDOutputFormat.js will be significantly shorter. Not sure if we care about adding extra bytes to each bundle here.

3. I have named function arguments in AMD prelude and worry about possible collisions between them and variables in existing source code. While I understand that there is no way to avoid collisions holistically, is there any established pattern used across codebase which will help minimize such collisions?

[devongovett]

Thanks for working on this!

1. Yes, we generally prefer integration tests. This lets us more easily change the implementation without breaking the tests (we reused a lot of tests from v1 in v2). The output-formats.js file is the right place.
2. I don't mind complexity in Parcel if it makes for smaller output. But I'll leave this up to you for the initial implementation. We can always optimize later if you want.
3. All top-level variable names within the generated code will already have been renamed to something like $assetId$var$xyz, so it should generally be safe to use variable names that don't fall under that pattern. The pattern you've used looks ok, though you could perhaps use this.packager.bundle.publicId as the prefix rather than the main asset id. Sometimes there is no main asset, e.g. in the case of shared bundles.

[bhovhannes]

Thank you for review @devongovett, and for your work on Parcel! You did a huge amount of work there.

1. Yes, we generally prefer integration tests. This lets us more easily change the implementation without breaking the tests (we reused a lot of tests from v1 in v2). The output-formats.js file is the right place.

ok, I'll add test there.

2. I don't mind complexity in Parcel if it makes for smaller output. But I'll leave this up to you for the initial implementation. We can always optimize later if you want.

ok, I'll add tests covering that as well.

3. All top-level variable names within the generated code will already have been renamed to something like $assetId$var$xyz, so it should generally be safe to use variable names that don't fall under that pattern. The pattern you've used looks ok, though you could perhaps use this.packager.bundle.publicId as the prefix rather than the main asset id. Sometimes there is no main asset, e.g. in the case of shared bundles.

That is very helpful, I'll change to use this.packager.bundle.publicId. Thanks!

[bhovhannes]

Hi @devongovett , @mischnic , Happy New Year!

I've added tests to AMD output format implementation, so this MR is ready for final review.

I was not able to find a way to create a sample repo where Parcel produces shared bundle for the 2 JS entries. Producing shared chunks works for HTML entries (like is written here), but I didn't find a way to generate shared chunks for JS entries. Thus, I was not able to test that case, although I think I covered that case by checking for empty main asset it and short-circuiting if so.
Currently, only the main asset code will be wrapped in AMD wrapper, I exclude dynamically loaded chunks as well because they are loaded by parcelRequire.

I think I also need to mention AMD format in https://parceljs.org/features/targets/#outputformat. What is the repo behind Parcel docs website so I can send an MR there as well? found it, never mind.

[bhovhannes]

Friendly ping 🙂

[oe]

any news?

[jamenai]

Any news here?