Skip to content

Commit

Permalink
Warn on mixing ESM and commonJS (#3807)
Browse files Browse the repository at this point in the history
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.

Co-authored-by: Théo Crevon <oleiade@users.noreply.github.com>
Co-authored-by: Ivan <2103732+codebien@users.noreply.github.com>
  • Loading branch information
3 people authored Jun 25, 2024
1 parent 8efec40 commit a525231
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 1 deletion.
12 changes: 11 additions & 1 deletion js/compiler/compiler.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,17 @@ 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(
"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 use solely one or the other syntax.",
filename)
}
return prg, code, err
}

if compatibilityMode == lib.CompatibilityModeExperimentalEnhanced {
Expand Down
23 changes: 23 additions & 0 deletions js/compiler/compiler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 has been detected that the file combines`)
}

0 comments on commit a525231

Please sign in to comment.