Add esm format alias #2102

TrySound · 2018-03-31T10:05:34Z

Ref #1917

Added esm as an alias to be compatible in naming with esm loader.

Also since it's more descriptive renamed a few variables too.

Should I add format for every form in tests?

guybedford

Thanks for getting to this, have been meaning to myself.

I guess we should decide if we want to make esm the main form and deprecate es or just allow it as an alias.

Perhaps just treating it as an alias for now, to avoid documentation changes etc might be simpler, but I'm open to considering the more major deprecation path - although will likely need further feedback from @Rich-Harris and @lukastaegert on this.

guybedford · 2018-03-31T12:31:21Z

src/rollup/index.ts

@@ -229,7 +229,7 @@ function checkOutputOptions(options: OutputOptions) {

 	if (!options.format) {
 		error({
-			message: `You must specify options.format, which can be one of 'amd', 'cjs', 'system', 'es', 'iife' or 'umd'`,
+			message: `You must specify options.format, which can be one of 'amd', 'cjs', 'system', 'esm', 'iife' or 'umd'`,


If we do this, we should work on a documentation change as well.

Although for now, I'm thinking it might just be better to treat esm as an alias, with es remaining the primary term - moving to consider this being changed in future?

guybedford · 2018-03-31T12:34:31Z

src/rollup/index.ts

@@ -229,7 +229,7 @@ function checkOutputOptions(options: OutputOptions) {

 	if (!options.format) {


An alternative here might be to do if (options.format === 'es') options.format = 'esm'; to avoid the need to do both checks through the codebase.

I guess this merge should happen in mergeOptions?

This could work, but there is an issue, what we should pass to transformBundle(code, { format })?

It would be a breaking change to alter - so keeping es internally and esm as an alias is probably the best bet.

guybedford · 2018-03-31T12:36:17Z

src/Chunk.ts

@@ -403,10 +403,10 @@ export default class Chunk {
 	}

 	private prepareDynamicImports({ format }: OutputOptions) {
-		const es = format === 'es';
+		const esm = format === 'esm' || format === 'es';


Ideally all these checks should only check one format string - either esm or es, with internal rewriting to ensure only one is used.

TrySound · 2018-03-31T14:14:49Z

Added tests for esm with a couple of hacks to reuse es expected

Rich-Harris · 2018-03-31T17:49:56Z

I'd be pro-deprecating es in favour of esm. I think it's much more descriptive for most people

TrySound · 2018-03-31T19:11:16Z

We can then merge this pr with an alias to give users time to migrate then add deprecation.

lukastaegert · 2018-03-31T20:12:40Z

I would prefer esm over es as well but I do not see a safe way to change that without potentially breaking important plugins. This not only affects the transformBundle hook but also the ongenerate and onwrite hooks and will require careful communication from our side that important plugins need to match both aliases before we can do the change. Not sure which plugins are actually affected, though.

TrySound · 2018-03-31T20:23:03Z

I can coordinate this breaking change through all of these modules.

guybedford

Looks great to me, just a question re possible duplication in the tests.

guybedford · 2018-04-01T15:14:55Z

test/form/index.js

@@ -6,7 +6,7 @@ const { extend, loadConfig, normaliseOutput } = require('../utils.js');

 const samples = path.resolve(__dirname, 'samples');

-const FORMATS = ['amd', 'cjs', 'system', 'es', 'iife', 'umd'];
+const FORMATS = ['amd', 'cjs', 'system', 'es', 'esm', 'iife', 'umd'];


Did you mean to include both es and esm here or just use esm?

Both. Esm is an alias which we also should test

Yes, but it will slow down the tests if we're rerunning each and every es module format test twice now. Perhaps just one test is necessary.

guybedford · 2018-04-01T15:15:42Z

test/chunking-form/index.js

@@ -7,7 +7,7 @@ const { extend, loadConfig } = require('../utils.js');

 const samples = path.resolve(__dirname, 'samples');

-const FORMATS = ['es', 'cjs', 'amd', 'system'];
+const FORMATS = ['es', 'esm', 'cjs', 'amd', 'system'];


Remove 'es' case?

We are gonna support both for now

guybedford · 2018-04-01T15:17:05Z

src/rollup/index.ts

@@ -229,7 +229,7 @@ function checkOutputOptions(options: OutputOptions) {

 	if (!options.format) {
 		error({
-			message: `You must specify options.format, which can be one of 'amd', 'cjs', 'system', 'es', 'iife' or 'umd'`,
+			message: `You must specify options.format, which can be one of 'amd', 'cjs', 'system', 'esm', 'es', 'iife' or 'umd'`,


Shall we already move to only documenting esm perhaps as a first step towards deprecation?

guybedford · 2018-04-17T12:36:03Z

Could you rebase this for merge? Otherwise I can aim to take a look.

Sorry this one dropped off my radar, would be great to get it in.

TrySound · 2018-04-17T19:53:11Z

Done

guybedford · 2018-04-17T20:42:24Z

Amazing, thanks for the update here.

We should be able to switch around the es and esm internally on a future major, deprecating the es input from there. Perhaps a 1.0 goal if we get that milestone going soon!

stefanocke · 2019-07-14T12:37:44Z

The generateBundle hook in rollup version 1.16 still gives me "es" instead of "esm" in outputOptions.format. In my rollup config, I have "esm".

Does this mean, the breaking change did not happen in 1.x?
Is it planned for future majos releases?

TrySound · 2019-07-14T12:38:57Z

@stefanocke Yes, breaking change did not happen before 1.0. esm is only alias for now. We may reconsider this later.

stefanocke · 2019-07-14T12:40:19Z

@TrySound , thanks for clarification.

mrdoob#17099 and rollup/rollup#2102

TrySound requested review from guybedford and lukastaegert March 31, 2018 10:05

guybedford requested changes Mar 31, 2018

View reviewed changes

TrySound force-pushed the esm-format-alias branch from af21c79 to eec7c39 Compare March 31, 2018 13:40

guybedford approved these changes Apr 1, 2018

View reviewed changes

TrySound added 4 commits April 17, 2018 21:33

Add esm format alias

491a432

Reduce checks and save 'es' in error

f58f28c

Test esm forms skipping source maps

1e83337

Remove esm tests from forms and add simple case

1958970

TrySound force-pushed the esm-format-alias branch from 2e67202 to 1958970 Compare April 17, 2018 19:50

guybedford merged commit e49ed10 into master Apr 17, 2018

TrySound deleted the esm-format-alias branch April 17, 2018 20:43

lukastaegert added this to the 0.59.0 milestone Apr 18, 2018

lukastaegert added the fixed pending release label Apr 18, 2018

TrySound mentioned this pull request May 13, 2018

Replace es with more descriptive esm Andarist/cherry-pick#1

Open

lukastaegert removed the fixed pending release label May 15, 2018

TrySound mentioned this pull request May 26, 2018

Prefer esm over es bvaughn/react-window#3

Merged

EliasHasle mentioned this pull request Jul 25, 2019

Is Rollup format "es" valid? mrdoob/three.js#17099

Closed

EliasHasle added a commit to EliasHasle/three.js that referenced this pull request Jul 25, 2019

Change format from es to esm

aba45cb

mrdoob#17099 and rollup/rollup#2102

EliasHasle mentioned this pull request Jul 25, 2019

Change rollup output format specifier for modules from es to esm mrdoob/three.js#17100

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add esm format alias #2102

Add esm format alias #2102

TrySound commented Mar 31, 2018

guybedford left a comment

guybedford Mar 31, 2018

guybedford Mar 31, 2018

guybedford Mar 31, 2018

guybedford Mar 31, 2018

TrySound Mar 31, 2018

guybedford Mar 31, 2018

guybedford Mar 31, 2018

TrySound commented Mar 31, 2018

Rich-Harris commented Mar 31, 2018

TrySound commented Mar 31, 2018

lukastaegert commented Mar 31, 2018

TrySound commented Mar 31, 2018

guybedford left a comment

guybedford Apr 1, 2018

TrySound Apr 1, 2018

guybedford Apr 1, 2018

guybedford Apr 1, 2018

TrySound Apr 1, 2018

guybedford Apr 1, 2018

TrySound Apr 1, 2018

guybedford commented Apr 17, 2018

TrySound commented Apr 17, 2018

guybedford commented Apr 17, 2018

stefanocke commented Jul 14, 2019 •

edited

Loading

TrySound commented Jul 14, 2019

stefanocke commented Jul 14, 2019

		@@ -229,7 +229,7 @@ function checkOutputOptions(options: OutputOptions) {

		if (!options.format) {

Add esm format alias #2102

Add esm format alias #2102

Conversation

TrySound commented Mar 31, 2018

guybedford left a 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

TrySound commented Mar 31, 2018

Rich-Harris commented Mar 31, 2018

TrySound commented Mar 31, 2018

lukastaegert commented Mar 31, 2018

TrySound commented Mar 31, 2018

guybedford left a 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

guybedford commented Apr 17, 2018

TrySound commented Apr 17, 2018

guybedford commented Apr 17, 2018

stefanocke commented Jul 14, 2019 • edited Loading

TrySound commented Jul 14, 2019

stefanocke commented Jul 14, 2019

stefanocke commented Jul 14, 2019 •

edited

Loading