-
Notifications
You must be signed in to change notification settings - Fork 4
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
Add support for modern browserslist environment #88
Merged
billyjanitsch
merged 35 commits into
kensho-technologies:main
from
nikita-kensho:nikita/modern-module
Apr 26, 2021
Merged
Add support for modern browserslist environment #88
billyjanitsch
merged 35 commits into
kensho-technologies:main
from
nikita-kensho:nikita/modern-module
Apr 26, 2021
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
8e6e320
to
da14810
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great! We also need to update the readme docs to mention the new envs.
I'm going to patch this and mess around with the browserslist config file in the test suite, because it's surprising to me that it stops working in the fixtures directory.
This reverts commit 41bdc8f.
ff978b7
to
ca43c7f
Compare
billyjanitsch
approved these changes
Apr 26, 2021
Merged
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Proposal for a new babel environment type to support compilation to a modern browser subset. These changes are intended to be backwards-compatible with existing builds.
How
These changes take advantage of babel's integration with the browserslist tool, and specify new babel environments allowed by our preset. This allows the user to create browser groups in their
.browserslistrc
config file, and instruct our babel preset to target these groups by passing one of the new environment types.Details
Babel's
preset-env
's default configuration will select the[production]
browsers group when it is defined. This allows us to be backwards-compatible with.browserslistrc
configs that do not contain any groups, and have projects upgrade by grouping the old browsers list under the[production]
group, while creating a new[production-modern]
group for modern browsers. The modern group can then be targeted using the newproduction-modern
ordevelopment-modern
babel environments.Note: the repository's
.browserslistrc
is strictly for testing and should be excluded from the published package. Unfortunately the preset was unable to run without the config file being at the root of the directory.Snapshot Verification
The last couple of commits contain incremental updates to the test snapshots. Below is an overview of the options being tested, a bit of relevant information to verifying that the updates are correct, and the set of changes.
overview of some of the babel options verified
{...obj}
operator. This should not be available for the defaultproduction
anddevelopment
builds, but active for other environmentssetImmediate
is not supported for most environments.targets
-- when set to modern browsers, this forces legacy environments to output ES2020 syntaxloose
-- whenfalse
, forces the preset to not to use assumptions that do not work in all cases; this disables shorthands such as== null
, enforces the use ofdefineProperty
for object/class assignments, and the assumption that all iterables can be treated as arrays (eg spreading a Map's key-val iterator will not result in invalid array methods being used on the iterator) TODO: add test for thismodules
-- changes the import style to require when set tocomonjs
runtime
-- whenfalse
, prevents babel from importing utility functions from shared source files, and forces babel to inline the source code into the outputreact: {runtime: 'classic'}
-- forces import of a unified React object, instead of importing individual React utilitiesemotion
-- enables the emotion styling functionalitypoc for targeting a browser group
This iteration had the test file limited so that the only environments and settings active were for legacy and modern browser groups. The relevant snapshot lines are below:
production with modern targets specified via plugin configuration:
babel-preset/__snapshots__/test.ts.snap
Line 27 in 023732b
production-modern which is integrated with .browserslistrc:
babel-preset/__snapshots__/test.ts.snap
Line 146 in 023732b
production with default settings:
babel-preset/__snapshots__/test.ts.snap
Line 115 in 023732b
The important thing to note is that the first 2 should be identical and contain syntax such as the spread operator, while the last should match the old settings instead. Presence of ES2020 syntax can be verified by looking for the spread operator. Also, nullish coalescing and optional chaining are not supported by webpack@4, so these operators are always compiled to legacy mode at the moment.
baseline
Diff of commit against main.
main...770fabc
Using the new babel code and grouping old browsers under
[production]
, while maintaining the same set of test fixtures, this diff show that no changes are made to the original snapshots, which means backwards compatibility is maintained.new environment snapshots
Note: when looking at this diff, split diff mode is less worse, but it is still messed up for some of the tests. Diffing locally using a better algorithm is better; and also I've found that the diff algorithm used by VS Code worked perfectly when verifying snapshot updates. The reason github is confused by diff is ... idk, but I think that because the new environments produce very similar code, the code they produced is mapped to the old environments' code, and github appears to show that old environments fixtures being updated instead. See link: 5d0485d#diff-69e445ad9ee632101f68a08ec5e14ab23851ae1938f7a1fa4f7c1954684e1cc9R2433-R2434
Update --
git diff --patience 5d0485d0bc068de8093c475d81765971bb1f5d68~ 5d0485d0bc068de8093c475d81765971bb1f5d68
produces a good diff for this commit.New environments are added to the test suite --
development-modern', 'production-modern', 'test'
-- with'test'
being an existing environment that was not covered by the original snapshots. The snapshot diff is linked: 5d0485dTo sanity-check these tests, you can follow the rules outline above and compare to the previous fixture code. For comparison against basic syntax transpilation, the output using default options should be looked at first, linked here:
babel-preset/__snapshots__/test.ts.snap
Lines 1188 to 1350 in fe00514
Generally, modern browser output will be similar to the existing
esm
environment, and thetest
output similar to thecommonjs
environment. Note that comparing thetest
environment is not necessary since it is removed in the next iteration.removing the
test
environment from testsSince our CI checks that we can run this preset with different node versions, and the
test
environment always compiles using the current node version, a single snapshot will not always satisfy both CI runs, so it is removed from the default environment set and run separately with a specific target node version. You can see the snapshot for the newtest
env below:f3766ea
TODO: do we want to replicate the other fixtures & param combos for
test
env?adding a new param combo:
{loose: false, runtime: false}
This configuration is added in order to force the modern environment fixtures to use babel-injected helper function. Since previously the
{runtime: false}
configuration was identical to a{}
configuration with modern environments, we needed a way to verify that this parameter is compiled correctly for them.{loose: false}
forces the transpilation of certain modern syntax to babel-based functions, which are then either imported or inlined if runtime is false, as the diff shows:c11a9c4
And to verify that the correct functions are inlined, the new code can be compared against the plain
{loose: false}
output (linking todevelopment-modern
below):babel-preset/__snapshots__/test.ts.snap
Lines 860 to 892 in c11a9c4
adding async/await and generators
Now that we can target modern environments, we don't have to worry about creating huge bundles for the majority of users. This allows us to use verbosely transpiled features in our source code. This commit adds tests for them, modifying existing syntax snapshots.
aa70ac1
TODO: do want to mutate the fixture code? Adding a class method generator forced a mutation of the class's transpiled code, which makes things a bit messier.
add nullish assignment test
This feature needs to be transpiled while we're on webpack@4. It's added to the list of operators being tested.
2acd1e6