Use Babel 7.x; Extract "babel" package; Update "browsers" default #639

lukeed · 2018-09-07T18:47:00Z

Not ready for merge – must resolve pre-existing Jest failures first.

What kind of change does this PR introduce?

Refactor, Feature

Did you add tests for your changes?

No, they're already a mess (again).

Summary

Better browser list
Upgraded Babel deps from beta to 7.x stable
Extracted Preact-specific config into @preact/babel package
Applied CLI-specific Babel config changes within CLI:
- Read & disable babelrc
- Attach fast-async support (Add fast-async babel plugin by default #617)
Fixed my typo artifact (old ~> .babelrc)

Does this PR introduce a breaking change?

The default browsers changed, dropping default support for legacy browsers.. so technically, "yes" but this is going to be a major bump already.

The Babel behavior should be unchanged; if anything, it'd be beta to 7.x changes.

This allows non-CLI users to download & designate @preact/babel as a Babel preset.

GitHub killed my original PR notes... wut

marvinhagemeister · 2018-09-07T19:06:59Z

@lukeed Really appreciate this. Was looking for a dedicated preact babel preset to recommend the other day that works with babel v7 👍

lukeed · 2018-09-07T19:10:31Z

Me too! I want to slice up what I have on PWA's preset-preact (and 7.x here) into these bite-sized chunks.

Repeating config over & over again across build chains is silly

reznord · 2018-09-07T19:46:54Z

packages/cli/tests/images/create.js

-	'src/tests/header.test.js'
+	'tests/__mocks__/browserMocks.js',
+	'tests/__mocks__/fileMocks.js',
+	'tests/header.test.js'


You fixed my PR changes as well 😛

Closing #631 since you made these changes

ForsakenHarmony · 2018-09-07T19:54:54Z

packages/cli/lib/lib/webpack/transform-config.js

-		presets: [[require.resolve('@babel/preset-env'), {
-			"targets": { "node": "current" }
-		}]]
-	});


This will break preact.config.js pretty much everywhere, are we gonna change something about that as well?

Oh, derp. I've always done module.exports and forgot we allowed that. If it were up to me, we'd drop it entirely.

Will add it back.

ForsakenHarmony · 2018-09-07T19:55:52Z

packages/cli/lib/lib/webpack/webpack-base-config.js

+		if (Array.isArray(x) && x[0].includes('@babel/preset-env')) {
+			x[1].exclude.push('transform-async-to-generator');
+		}
+	});


also not sure if this is the best idea, especially if someone wants to extend the config and then async breaks for them

If they're overwriting, it gets lost anyway. The extend/overwrite happens after this point.. same as before

but if you extend the config you won't have fast-async because that will override the default one here

Fast async isn't included in preact preset because it's not a part of getting Preact to run. It's a CLI feature not a preact requirement

yes, but now if someone makes a custom .babelrc and extends from @preact/babel they'll get errors

in theory you are changing the behaviour from before though, fast-async was in the extendable config from the cli

With this package, one can do this:

{ "presets": [ "@preact/babel", ["@babel/preset-env", { "exclude": [ "transform-async-to-generator" ] }] ], "plugins": [ ["fast-async", { "spec": true }] ] }

And they'd be at the same settings. In ^this state, it's a pointless .babelrc file, but ofc this assumes the user had something worth doing.

is that gonna take the rest of the preset-env config from @preact/babel?

Yes @preact/babel is this entire file: https://github.com/developit/preact-cli/blob/6ba1267cb8ffbbeb94705ac6cef246bc075db161/packages/babel/index.js

Everything else you seem to be concerned about is "injected" into @preact/babel by the CLI immediately after loading it.

so the only thing I want to know is if your example .babelrc would basically get turned into this by babel

["@babel/preset-env", { "loose": true, "modules": false, "targets": { "browsers": [ ">0.25%", "not ie 11'" "not op_mini all" ] } "exclude": [ "transform-regenerator", "transform-async-to-generator", "transform-typeof-symbol" ] }]

and our config doesn't get completely overriden

ForsakenHarmony · 2018-09-07T20:17:52Z

packages/cli/package.json

@@ -46,6 +50,7 @@
  },
  "dependencies": {
    "@babel/core": "^7.0.0",
+    "@babel/register": "^7.0.0",


what about @babel/preset-env? 👀

Babel package

Oh I see what you're saying. It'd resolve but will add to be safe

# Conflicts: # packages/babel/index.js # packages/cli/babel/index.js # packages/cli/lib/lib/webpack/transform-config.js # packages/cli/lib/lib/webpack/webpack-base-config.js # packages/cli/package.json # packages/cli/tests/images/create.js # packages/cli/yarn.lock # yarn.lock

lukeed · 2019-01-25T23:49:11Z

Does this pass @ForsakenHarmony inspection now?

ForsakenHarmony · 2019-01-25T23:57:00Z

still don't know how I feel about fast-async being excluded

also have babel-plugin-macros in there now 👀

I feel like it's best to just have this as like cli-babel-config or smth

tomasdev · 2019-01-29T23:19:08Z

IE11 has more user share than Edge 17/18 as the time of writing this. I would not recommend going with that "better" not so better coverage.

ForsakenHarmony · 2019-01-30T01:08:54Z

the difference is, ie 11 keeps us back a lot further than edge, which with that browser list probably doesn't even get special treatment

tomasdev · 2019-01-30T02:19:39Z

I completely understand that, then the question I should have asked is: what's the purpose of preact-cli? towards production applications used by corporations, or more towards prototypes future-thinking that don't care about usage?
Of course it's probably none of the extremes, and in the end it's configurable so... up to the owners. I just threw my opinion.

lukeed added 7 commits September 7, 2018 10:30

update to babel@7 packages (non-beta)

793eca2

remove @babel/register

7704f87

fix: look for .babelrc file

542a4a8

add preact/babel package

bb37beb

use preact/babel to replace inline babel config

30cf8b9

update locks

c6315e8

update create snapshot for "default" template

bc091df

lukeed changed the title ~~Add "preact/babel~~ Use Babel 7.x; Extract "babel" package; Update "browsers" default Sep 7, 2018

lukeed mentioned this pull request Sep 7, 2018

Use preact and preact-router from source instead of dist #636

Open

reznord reviewed Sep 7, 2018

View reviewed changes

ForsakenHarmony requested changes Sep 7, 2018

View reviewed changes

ForsakenHarmony reviewed Sep 7, 2018

View reviewed changes

fix: allow ESM preact.config.js file

569b990

ForsakenHarmony reviewed Sep 7, 2018

View reviewed changes

redeclare babel/preset-env dep

6ba1267

lukeed mentioned this pull request Sep 15, 2018

Fix Tests #648

Merged

ForsakenHarmony added 3 commits January 25, 2019 23:23

update lockfile

666d7b4

remove babelrc from the snapshot

70208b7

ForsakenHarmony changed the base branch from next to master March 19, 2019 19:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Use Babel 7.x; Extract "babel" package; Update "browsers" default #639

Use Babel 7.x; Extract "babel" package; Update "browsers" default #639

lukeed commented Sep 7, 2018 •

edited

Loading

marvinhagemeister commented Sep 7, 2018

lukeed commented Sep 7, 2018

reznord Sep 7, 2018

reznord Sep 7, 2018 •

edited

Loading

ForsakenHarmony Sep 7, 2018

lukeed Sep 7, 2018

ForsakenHarmony Sep 7, 2018

lukeed Sep 7, 2018

ForsakenHarmony Sep 7, 2018

lukeed Sep 7, 2018

ForsakenHarmony Sep 7, 2018

ForsakenHarmony Sep 7, 2018

lukeed Sep 7, 2018

ForsakenHarmony Sep 7, 2018

lukeed Sep 7, 2018

ForsakenHarmony Sep 12, 2018

ForsakenHarmony Sep 7, 2018

lukeed Sep 7, 2018

lukeed Sep 7, 2018

lukeed commented Jan 25, 2019

ForsakenHarmony commented Jan 25, 2019

tomasdev commented Jan 29, 2019

ForsakenHarmony commented Jan 30, 2019

tomasdev commented Jan 30, 2019

Use Babel 7.x; Extract "babel" package; Update "browsers" default #639

Are you sure you want to change the base?

Use Babel 7.x; Extract "babel" package; Update "browsers" default #639

Conversation

lukeed commented Sep 7, 2018 • edited Loading

What kind of change does this PR introduce?

Did you add tests for your changes?

Summary

Does this PR introduce a breaking change?

marvinhagemeister commented Sep 7, 2018

lukeed commented Sep 7, 2018

Choose a reason for hiding this comment

reznord Sep 7, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

lukeed commented Jan 25, 2019

ForsakenHarmony commented Jan 25, 2019

tomasdev commented Jan 29, 2019

ForsakenHarmony commented Jan 30, 2019

tomasdev commented Jan 30, 2019

lukeed commented Sep 7, 2018 •

edited

Loading

reznord Sep 7, 2018 •

edited

Loading