Skip to content
This repository has been archived by the owner on Mar 4, 2020. It is now read-only.

fix(plugins): use compiled dependency instead of css-expand-shorthand #1542

Merged
merged 5 commits into from
Jun 25, 2019

Conversation

layershifter
Copy link
Member

@layershifter layershifter commented Jun 25, 2019

Fixes #1534.

Problem

IE11 does not have support of arrow functions while our dependency css-shorthand-expand has its usage. And for client there is only one option to fix - compile these dependency with babel-loader or any other thing 👎

Solution

I used @pika/pack to quicky get bundled library, so our cssExpandShorthand.ts contains css-shorthand-expand and all its dependencies in the single and tree-shaken file. As we compile all our files with Babel: arrow functions and any other unsupported things will be compiled properly 🚀

@@ -32,24 +31,14 @@
</style>
</head>
<body>
<script src="https://cdn.jsdelivr.net/npm/@babel/polyfill@7/dist/polyfill.min.js"></script>
<script src='https://cdn.jsdelivr.net/npm/core-js-bundle/minified.js'></script>
<script src='https://cdn.jsdelivr.net/npm/whatwg-fetch/dist/fetch.umd.min.js'></script>
Copy link
Member Author

Choose a reason for hiding this comment

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

fetch() is not supported by IE11

@@ -32,24 +31,14 @@
</style>
</head>
<body>
<script src="https://cdn.jsdelivr.net/npm/@babel/polyfill@7/dist/polyfill.min.js"></script>
<script src='https://cdn.jsdelivr.net/npm/core-js-bundle/minified.js'></script>
Copy link
Member Author

Choose a reason for hiding this comment

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

core-js-bundle is bundled version of core-js which is a successor for @babel/polyfill:
https://babeljs.io/docs/en/babel-polyfill

<script
crossOrigin="anonymous"
src="https://unpkg.com/prettier@<%= htmlWebpackPlugin.options.versions.prettier %>/parser-typescript.js"
src="https://cdn.jsdelivr.net/combine/npm/prettier@<%= htmlWebpackPlugin.options.versions.prettier %>/standalone.min.js,npm/prettier@<%= htmlWebpackPlugin.options.versions.prettier %>/parser-babylon.min.js,npm/prettier@<%= htmlWebpackPlugin.options.versions.prettier %>/parser-html.min.js,npm/prettier@<%= htmlWebpackPlugin.options.versions.prettier %>/parser-typescript.min.js"
Copy link
Member Author

Choose a reason for hiding this comment

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

Instead of 4 files unminified files from CDN we will require only minified one 🚤

@@ -62,7 +51,8 @@

<script src="https://cdn.jsdelivr.net/npm/@babel/standalone@<%= htmlWebpackPlugin.options.versions.babelStandalone %>/babel.min.js"></script>

<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/prismjs@1.16.0/themes/prism-tomorrow.css">
<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/prismjs@1.16.0/themes/prism-tomorrow.min.css">
<link rel="stylesheet" href="https://cdn.jsdelivr.net/npm/react-vis@<%= htmlWebpackPlugin.options.versions.reactVis %>/dist/style.min.css" />
Copy link
Member Author

Choose a reason for hiding this comment

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

Also minified versions

@codecov
Copy link

codecov bot commented Jun 25, 2019

Codecov Report

Merging #1542 into master will decrease coverage by 1.34%.
The diff coverage is 54.14%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1542      +/-   ##
==========================================
- Coverage   73.16%   71.82%   -1.35%     
==========================================
  Files         828      829       +1     
  Lines        6227     6697     +470     
  Branches     1786     1898     +112     
==========================================
+ Hits         4556     4810     +254     
- Misses       1666     1882     +216     
  Partials        5        5
Impacted Files Coverage Δ
packages/react/src/lib/felaRenderer.tsx 66.66% <ø> (ø) ⬆️
...ges/react/src/lib/felaExpandCssShorthandsPlugin.ts 100% <ø> (ø) ⬆️
packages/react/src/lib/cssExpandShorthand.ts 54.14% <54.14%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 0a9e616...de55a11. Read the comment docs.

zIndex: 100,
}}
title={copyable ? 'Copy' : undefined}
>
<div>{label || mode}</div>
{copyable && <div style={{ marginLeft: '5px' }}>{active ? '✔ ' : '⎘'}</div>}
{copyable && <div style={{ marginLeft: '5px' }}>{active ? checkIcon : copyIcon}</div>}
Copy link
Member Author

Choose a reason for hiding this comment

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

image

Current icon looks ugly and there are no good replacement, so I decided to use SVGs.
Also it's not enough contrast, so change color to #ccc from PrismJS theme.

After

codesnippet-icons

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.

[IE11] Dependency js file has arrow function
2 participants