Skip to content
This repository has been archived by the owner on Sep 21, 2022. It is now read-only.

fix: remove polyfill-service dependency #674

Merged
merged 1 commit into from
Nov 18, 2016
Merged

fix: remove polyfill-service dependency #674

merged 1 commit into from
Nov 18, 2016

Conversation

SevInf
Copy link
Contributor

@SevInf SevInf commented Nov 16, 2016

Fixes a lot of warnings regarding outdated dependecies and allows
to install gemini with yarn.

How its done:

  • query client module is replaced by lib module which will map either to native functions or to polyfills, similar to how querySelector/Sizzle sepatation was done before.
  • during calibration we'll determine if browser supports all modern features we need (css3 selectors, getComutedStyle, matchMedia, String::trim).
  • if any of them is missing, compatibility library will be used. Which means, polyfills will be used for all of them. In theory, it is less granular than the previous approach, in practice it means that out of currently supported browsers polyfills will be used only for IE8

Close #673, close #635

/cc @sipayRT

function hasCSS3Selectors() {
try {
document.querySelector('body:nth-child(1)');
} catch (e) {
Copy link
Member

Choose a reason for hiding this comment

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

линтер должен тут ругнуться

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Почему? Это ж по сути тоже самое, что и до этого было.

Copy link
Member

Choose a reason for hiding this comment

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

ну типа аргумент, который потом не используется

Copy link
Contributor Author

@SevInf SevInf Nov 16, 2016

Choose a reason for hiding this comment

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

Ну это не аргумент функции, без него код не распарсится

return str.replace(/^[\s\uFEFF\xA0]+|[\s\uFEFF\xA0]+$/g, '');
};

exports.getComputedStyle = require('./polyfills/getComputedStyle');
Copy link
Member

Choose a reason for hiding this comment

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

require('./polyfills/getComputedStyle').getComputedStyle
ниже строка аналогично

};

exports.trim = function(str) {
return str.replace(/^[\s\uFEFF\xA0]+|[\s\uFEFF\xA0]+$/g, '');
Copy link
Member

@sipayRT sipayRT Nov 16, 2016

Choose a reason for hiding this comment

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

давай сюда коммент добавим - хрен поймешь что мы тут реплейсим

@@ -0,0 +1,114 @@
/**
* Adapted from: https://raw.githubusercontent.com/Financial-Times/polyfill-service
Copy link
Member

Choose a reason for hiding this comment

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

я так понял, что эти файлы ревьюить вообще не нужно?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Да, от оригинальных они отличаются только тем, что под browserify и наш кодстайл подогнаны.

@sipayRT
Copy link
Member

sipayRT commented Nov 16, 2016

у себя локально поправил lib.compat.js и прогнал тесты в ie8 - все работает

Fixes a lot of warnings regarding outdated dependecies and allows
to install gemini with yarn.

Close #673, close #635
@SevInf
Copy link
Contributor Author

SevInf commented Nov 17, 2016

@sipayRT 🆙

@sipayRT
Copy link
Member

sipayRT commented Nov 18, 2016

все ок, работает

@SevInf SevInf merged commit 364bcbf into master Nov 18, 2016
@SevInf SevInf deleted the fix/no-polyfill branch November 18, 2016 11:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

yarn install fails for gemini
2 participants