Exposing System.register _context variable to bundled code.

[joeldenning]

Motivation

The motivation for this change is discussed in systemjs/systemjs#1939.

The change allows users to do things like the following:

// Set the public path on the fly, depending on the dynamic URL that SystemJS downloaded
// this bundle from
const dynamicUrl = __system_context__.meta.url
__webpack_public_path__ = dynamicUrl.slice(0, dynamicUrl.lastIndexOf('/') + 1)

What kind of change does this PR introduce?

Feature. This allows webpack users to use the _context argument provided to System.register bundles.

Did you add tests for your changes?

Yes.

Does this PR introduce a breaking change?

No

What needs to be documented once your changes are merged?

We need to document that __system_context__ is a variable available to those bundling to output.libraryTarget: "system".

[webpack-bot]

For maintainers only:

• This need to be documented (issue in webpack/webpack.js.org will be filed when merged)

[webpack-bot]

Thank you for your pull request! The most important CI builds succeeded, we’ll review the pull request soon.

[guybedford]

This is a great low-level way to support SystemJS contextual metadata.

[sokra]

Your implementation only works in the main chunk. Non of the code-splitted chunks would have access to __system_context__.

[guybedford]

@sokra since the jsonp interface doesn't have an easy way to pipe through this __system_context__ argument, I think we'd actually get that support naturally if we upgrade the system output to use System.import to load the chunks over the webpack json internal loader (and that would reduce bundle size too).

So would make this PR dependent on having native SystemJS chunk loading support for webpack chunks, which we may not be able to implement any time soon.

It seems a shame to block this useful feature on the development of more difficult feature.

[joeldenning]

Your implementation only works in the main chunk.

Afaik, setting the public path dynamically is something you only really need to do in the main chunk. So I see value in the PR as is, and would be happy with it merged even with the limitation you described.

All of that said, what do you think of the following implementation for sharing __system_context__ between chunks:

// output bundle
System.register([], function(_export, __system_context__) {
  return {
    execute: function() {
      // normal webpack initialization stuff
      // ...
     
      // Add system context to webpack require
      __webpack_require__.__system_context__ = __system_context__
    }
  }
})

Is there a better way to go about sharing the system context? Or would it turn into a full blown code splits refactor, like you mentioned before?

[guybedford]

@sokra if you have a moment to give any advice on how we might move forward with this general direction, would really appreciate your thoughts.

[joeldenning]

@sokra I have updated this with your feedback.

[guybedford]

It would be great to get this supported.

[joeldenning]

Any update on this? I know you are busy and appreciate your time 😄

[jsf-clabot]

All committers have signed the CLA.

[joeldenning]

@sokra I have updated this with your feedback

[joeldenning]

I have signed the CLA but Github's checks section says I haven't. Do I need to trigger another build or something to make Github realize that it is signed?

[alexander-akait]

@joeldenning you email in commit is not same as in github

[webpack-bot]

@joeldenning Thanks for your update.

I labeled the Pull Request so reviewers will review it again.

@sokra Please review the new changes.

[joeldenning]

Bump. I just merged master into this branch. I would very much appreciate a review 😄.

[joeldenning]

Just merged master into this. Now fully up-to-date with latest master branch.

[joeldenning]

The code coverage report seems to be incorrect? It lists a lot of files that have not been changed in my pull request as the reason why code coverage is failing. For example, in this report it cites /lib/util/identifier as a file that now has worse coverage. However, that file does not appear in the this PR's Files Changed tab, and I did not modify it. This is only one example of many where the code coverage tool seems to be picking up the wrong changes and holding this PR accountable for things that it did not change.

I tried retriggering the build by pushing new commits, but to no effect. I also wonder whether I need to squash my commits to see if that helps it sort out what are the new changes and what aren't. I started this PR before the master branch became webpack@5, and so there are large merge commits that potentially could be causing trouble.

If anyone has advice on this, I'd love to hear it. Despite this PR being almost a year old, I'd still very much like to see it reviewed and merged, if possible.

[webpack-bot]

The minimum test ratio has been reached. Thanks!

[joeldenning]

@sokra (or anyone else with merge privilege) I would appreciate your review on this. It is ready for merge - the coverage failures seem to be unrelated.

[sokra]

Thanks