Skip to content

Fix regression where polyfills where build into into extensions.#1681

Merged
cramforce merged 1 commit intoampproject:masterfrom
cramforce:polyfills-extensions
Jan 29, 2016
Merged

Fix regression where polyfills where build into into extensions.#1681
cramforce merged 1 commit intoampproject:masterfrom
cramforce:polyfills-extensions

Conversation

@cramforce
Copy link
Copy Markdown
Member

No description provided.

@cramforce cramforce force-pushed the polyfills-extensions branch from 4818a81 to d9f5cbc Compare January 29, 2016 19:28
src/service.js Outdated
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can we remove the import in timers.js?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

We really need a better solution. Basically you need to put it everywhere that uses promises and has an immediate side effect.

@cramforce cramforce force-pushed the polyfills-extensions branch from d9f5cbc to 6c9efcc Compare January 29, 2016 20:45
@cramforce
Copy link
Copy Markdown
Member Author

PTAL

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think the conditional is backwards?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It isn't.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

There are two files that resolve to the module name. One is empty. If we should include polyfills, it removes the empty from the set, otherwise it removes the actual file.

@erwinmombay
Copy link
Copy Markdown
Member

@cramforce tested, LGTM

cramforce added a commit that referenced this pull request Jan 29, 2016
Fix regression where polyfills where build into into extensions.
@cramforce cramforce merged commit 9054298 into ampproject:master Jan 29, 2016
@cramforce cramforce deleted the polyfills-extensions branch January 29, 2016 21:40
Gregable pushed a commit that referenced this pull request Jul 18, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants