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

Modules support now requires dynamic import and import.meta #191

Merged
merged 13 commits into from
Apr 24, 2018

Conversation

aomarks
Copy link
Member

@aomarks aomarks commented Apr 20, 2018

Modules support now requires dynamic import and import.meta.

Specifically, this means:

  • Safari modules support is now from 11.1 instead of 10.1/10.3
  • Chrome modules support is now from 64 instead of 61.
  • Vivaldi supports modules (tested).

Specifically, this means:

- Safari no longer supports modules. Recent Technology Preview builds
have support, but I'm not confident about what user agent matching we
can rely on there.
- Chrome modules support is now from 64 instead of 61.
- Vivaldi supports modules (tested).

Part of #117
@aomarks aomarks requested a review from rictic April 20, 2018 03:54
@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

@rictic
Copy link
Contributor

rictic commented Apr 20, 2018

CLA status is good because it's just Al and me on this PR, and we're both googlers.

Looks like browser-capabilites tests are failing in a legit-looking way, saying that Chrome doesn't have module support? Maybe something's up where it's identifying the Chrome UA as Safari?

@rictic
Copy link
Contributor

rictic commented Apr 20, 2018

Oh, it's probably just because we pushed back the min Chrome version that we count as having modules support

@aomarks
Copy link
Member Author

aomarks commented Apr 20, 2018

Looks like browser-capabilites tests are failing in a legit-looking way, saying that Chrome doesn't have module support? Maybe something's up where it's identifying the Chrome UA as Safari?

Yeah I just forgot to update the test last night, working on it.

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

@rictic rictic added cla: yes and removed cla: no labels Apr 20, 2018
@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

@googlebot
Copy link

So there's good news and bad news.

👍 The good news is that everyone that needs to sign a CLA (the pull request submitter and all commit authors) have done so. Everything is all good there.

😕 The bad news is that it appears that one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that here in the pull request.

Note to project maintainer: This is a terminal state, meaning the cla/google commit status will not change from this state. It's up to you to confirm consent of the commit author(s) and merge this pull request when appropriate.

'Mozilla/5.0 (Macintosh; Intel Mac OS X 10_11) AppleWebKit/603.1.30 (KHTML, like Gecko) Version/10.1 Safari/603.1.30',
['es2015', 'push', 'modules']);
});

Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to keep this test (updated expected of course) and add a new one for the latest Safari's

Copy link
Member Author

Choose a reason for hiding this comment

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

Done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants