From 4bfeec268d9058644030c9c4f5ec462e7f56b21f Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Mon, 24 Jun 2024 11:12:37 +0300 Subject: [PATCH 1/5] Warn on mixing ESM and commonJS As part of testing for #3265 it was noticed this isn't uncommon. As there is no way for this to be supported and it will be against how the specification works we prefer to warn users on this at least for a little while. --- js/compiler/compiler.go | 10 +++++++++- js/compiler/compiler_test.go | 23 +++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/js/compiler/compiler.go b/js/compiler/compiler.go index f0f9d7acc11..65fa45e6d8f 100644 --- a/js/compiler/compiler.go +++ b/js/compiler/compiler.go @@ -251,7 +251,15 @@ func (c *Compiler) compileImpl( return nil, code, err } // the compatibility mode "decreases" here as we shouldn't transform twice - return c.compileImpl(code, filename, wrap, lib.CompatibilityModeBase, state.srcMap) + var prg *sobek.Program + prg, code, err = c.compileImpl(code, filename, wrap, lib.CompatibilityModeBase, state.srcMap) + if err == nil && strings.Contains(src, "module.exports") { + c.logger.Warningf( + "While compiling %q it was noticed that it mixes import/export syntax (ESM) and commonJS module.exports. "+ + "This isn't standard behaviour and will soon not work. Please use one or the other.", + filename) + } + return prg, code, err } if compatibilityMode == lib.CompatibilityModeExperimentalEnhanced { diff --git a/js/compiler/compiler_test.go b/js/compiler/compiler_test.go index 22c90503518..7f4cb6e89d8 100644 --- a/js/compiler/compiler_test.go +++ b/js/compiler/compiler_test.go @@ -241,3 +241,26 @@ func TestMinimalSourceMap(t *testing.T) { require.NoError(t, err) require.Empty(t, hook.Drain()) } + +func TestMixingImportExport(t *testing.T) { + t.Parallel() + logger := logrus.New() + logger.SetLevel(logrus.DebugLevel) + logger.Out = io.Discard + hook := testutils.NewLogHook(logrus.InfoLevel, logrus.WarnLevel) + logger.AddHook(hook) + + compiler := New(logger) + compiler.Options = Options{ + CompatibilityMode: lib.CompatibilityModeExtended, + Strict: true, + } + _, _, err := compiler.Compile("export let s = 5;\nmodule.exports = 'something';", "somefile", false) + require.NoError(t, err) + entries := hook.Drain() + require.Len(t, entries, 1) + msg, err := entries[0].String() // we need this in order to get the field error + require.NoError(t, err) + + require.Contains(t, msg, `it was noticed that it mixes`) +} From 60943482eb41a5f108c7df4938af98b60a9a1c3e Mon Sep 17 00:00:00 2001 From: Mihail Stoykov <312246+mstoykov@users.noreply.github.com> Date: Mon, 24 Jun 2024 11:33:26 +0300 Subject: [PATCH 2/5] Update js/compiler/compiler.go MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Théo Crevon --- js/compiler/compiler.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/js/compiler/compiler.go b/js/compiler/compiler.go index 65fa45e6d8f..233dd44b9b7 100644 --- a/js/compiler/compiler.go +++ b/js/compiler/compiler.go @@ -255,8 +255,8 @@ func (c *Compiler) compileImpl( prg, code, err = c.compileImpl(code, filename, wrap, lib.CompatibilityModeBase, state.srcMap) if err == nil && strings.Contains(src, "module.exports") { c.logger.Warningf( - "While compiling %q it was noticed that it mixes import/export syntax (ESM) and commonJS module.exports. "+ - "This isn't standard behaviour and will soon not work. Please use one or the other.", + "During the compilation of %q, it has been detected that the file combines ECMAScript modules (ESM) import/export syntax with commonJS module.exports. "+ + "Mixing these two module systems is non-standard and will not be supported anymore in future releases. Please ensure to solely one or the other syntax.", filename) } return prg, code, err From 141c7c1ec18bee73ff74040a24d5b77090162112 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Mon, 24 Jun 2024 11:34:37 +0300 Subject: [PATCH 3/5] Fix test --- js/compiler/compiler_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/compiler/compiler_test.go b/js/compiler/compiler_test.go index 7f4cb6e89d8..89b44cc971d 100644 --- a/js/compiler/compiler_test.go +++ b/js/compiler/compiler_test.go @@ -262,5 +262,5 @@ func TestMixingImportExport(t *testing.T) { msg, err := entries[0].String() // we need this in order to get the field error require.NoError(t, err) - require.Contains(t, msg, `it was noticed that it mixes`) + require.Contains(t, msg, `it has been detected that the file combines`) } From d47e7cfaa74e33b7459082ce5abdbf3a46468769 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Mon, 24 Jun 2024 11:36:21 +0300 Subject: [PATCH 4/5] fix linter --- js/compiler/compiler.go | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/js/compiler/compiler.go b/js/compiler/compiler.go index 233dd44b9b7..857878f3404 100644 --- a/js/compiler/compiler.go +++ b/js/compiler/compiler.go @@ -255,8 +255,10 @@ func (c *Compiler) compileImpl( prg, code, err = c.compileImpl(code, filename, wrap, lib.CompatibilityModeBase, state.srcMap) if err == nil && strings.Contains(src, "module.exports") { c.logger.Warningf( - "During the compilation of %q, it has been detected that the file combines ECMAScript modules (ESM) import/export syntax with commonJS module.exports. "+ - "Mixing these two module systems is non-standard and will not be supported anymore in future releases. Please ensure to solely one or the other syntax.", + "During the compilation of %q, it has been detected that the file combines ECMAScript modules (ESM) "+ + "import/export syntax with commonJS module.exports. "+ + "Mixing these two module systems is non-standard and will not be supported anymore in future releases. "+ + "Please ensure to solely one or the other syntax.", filename) } return prg, code, err From e7dc8e1e7b062cda8ab8fec17deb8f984d0fe10d Mon Sep 17 00:00:00 2001 From: Mihail Stoykov <312246+mstoykov@users.noreply.github.com> Date: Mon, 24 Jun 2024 12:10:37 +0300 Subject: [PATCH 5/5] Update js/compiler/compiler.go Co-authored-by: Ivan <2103732+codebien@users.noreply.github.com> --- js/compiler/compiler.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/compiler/compiler.go b/js/compiler/compiler.go index 857878f3404..27860a5cb9b 100644 --- a/js/compiler/compiler.go +++ b/js/compiler/compiler.go @@ -258,7 +258,7 @@ func (c *Compiler) compileImpl( "During the compilation of %q, it has been detected that the file combines ECMAScript modules (ESM) "+ "import/export syntax with commonJS module.exports. "+ "Mixing these two module systems is non-standard and will not be supported anymore in future releases. "+ - "Please ensure to solely one or the other syntax.", + "Please ensure to use solely one or the other syntax.", filename) } return prg, code, err