Skip to content

Conversation

@guybedford
Copy link
Contributor

This resolves #6359 using the method suggested by @vladima as the replacement to the original PR at #6098.

This context object is described at https://github.com/ModuleLoader/es6-module-loader/wiki/System.register-Explained#how-it-works, and was considered more extensible for future spec changes than a string second argument directly.

@msftclas
Copy link

Hi @guybedford, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

@msftclas
Copy link

@guybedford, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

@mhegazy
Copy link
Contributor

mhegazy commented Jan 28, 2016

👍, looks like we have two unit tests that need to be updated.
@vladima, can you take a look.

@vladima
Copy link
Contributor

vladima commented Jan 28, 2016

I think this variable has to be injected after the prologue directive "use strict"

Copy link
Contributor

Choose a reason for hiding this comment

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

this should be var __moduleName = ${contextObjectForFile} && ${contextObjectForFile}.id; to avoid breaking users of older JSPM versions.

@guybedford
Copy link
Contributor Author

Thanks, I've made these corrections.

@vladima
Copy link
Contributor

vladima commented Jan 29, 2016

@guybedford can you please fix two remaining failing tests so we can pull in this PR?

@guybedford
Copy link
Contributor Author

@vladima sure I've got them passing now.

mhegazy added a commit that referenced this pull request Jan 29, 2016
Set __moduleName from context.id argument
@mhegazy mhegazy merged commit 1ae7d5c into microsoft:master Jan 29, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Jan 29, 2016

thanks!

@mhegazy mhegazy added this to the TypeScript 1.8 milestone Jan 29, 2016
@mhegazy mhegazy added the Breaking Change Would introduce errors in existing code label Jan 29, 2016
@mhegazy
Copy link
Contributor

mhegazy commented Jan 29, 2016

@vladima can you merge this in release-1.8 as well.

vladima pushed a commit that referenced this pull request Jan 29, 2016
Set __moduleName from context.id argument
vladima added a commit that referenced this pull request Jan 29, 2016
@guybedford
Copy link
Contributor Author

👍 Thanks for all the help getting this in.

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Breaking Change Would introduce errors in existing code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

System.register module name update

4 participants