errors reported with onwarn

[ashi009]

• Rollup Version: v0.67.4
• Operating System (or Browser): macOS 10.14.1
• Node Version: v10.7.0

How Do We Reproduce?

https://rollupjs.org/repl?version=0.67.4&shareable=JTdCJTIybW9kdWxlcyUyMiUzQSU1QiU3QiUyMm5hbWUlMjIlM0ElMjJtYWluLmpzJTIyJTJDJTIyY29kZSUyMiUzQSUyMmltcG9ydCUyMColMjBhcyUyMGElMjBmcm9tJTIwJy4lMkZhJyUzQiU1Q24lNUNuYS55KCU1QyUyMmFiYyU1QyUyMiklMjIlN0QlMkMlN0IlMjJuYW1lJTIyJTNBJTIyYS5qcyUyMiUyQyUyMmNvZGUlMjIlM0ElMjJleHBvcnQlMjAlN0IlN0QlM0IlMjIlN0QlNUQlMkMlMjJvcHRpb25zJTIyJTNBJTdCJTIyZm9ybWF0JTIyJTNBJTIyaWlmZSUyMiUyQyUyMm5hbWUlMjIlM0ElMjJteUJ1bmRsZSUyMiUyQyUyMmFtZCUyMiUzQSU3QiUyMmlkJTIyJTNBJTIyJTIyJTdEJTdEJTJDJTIyZXhhbXBsZSUyMiUzQW51bGwlN0Q=

Expected Behavior

Reporting an error for not being able to find y and fail the build.

Actual Behavior

Success build and no error being reported when --silent is set, as the error message is being reported through onwarn.

[lukastaegert]

I understand your issue. The reason why accessing (and possibly calling) a non-exported variable on a namespace object is not an error is that this is actually valid JavaScript and might actually be intended in certain situations. If you want an error to be thrown, you can check the provided warning in onwarn and throw an error there.

[ashi009]

However, the generated code is hardly valid:

(function () {
	'use strict';

	undefined("abc");

}());

For a warning case, the original variables should be preserved instead of substituting them with undefined.

[lukastaegert]

It valid in the sense that it is parseable JavaScript and only throws a runtime exception. The behaviour is also 100% consistent with what would happen if you run this code directly in an ESM environment such as a modern browser. If instead of calling the missing export you would just check for its presence, this behaviour would make sense and have the correct result. We are of course aware that this is rarely what you what which is why you get the warning.

[lukastaegert]

The point is, there is no "original variable" as internally, the namespace does not exist. According to the spec, a missing export when accessed via a namespace object is equivalent to undefined.

[ashi009]

I understand your point. Assuming someone runs rollup with --silent flag, such warning/error will never be seen. As the process exits with 0 and this will be a surprising runtime error.

In a more practical context, one could get this as a result of not using CommonJS plugin to rollup a script with a CommonJS dependency. The original code works, but the rolled up version doesn't and it will take time to figure out the root cause.

As you have said such behavior is almost certainly to be undesired, why is still a warning instead of an error. As a comparison closure compiler will fail a build loudly in this kind of cases, unless certain flags are passed or source code is annotated to supress the error.

IMHO, rollup could introduce a strict mode to report all these errors as error and let the warning be just warnings.

[filipesilva]

As far as I can tell, even throwing inside onwarn is ignored by --silent. I opened a separate issue in #2686.

[lukastaegert]

Thinking about this issue more (there have been similar discussions about circular dependencies, albeit here people were looking for a way to get rid of some warnings), maybe an option to filter warnings would be a good way forward. Here is an initial suggestion:

Elevate some warnings to errors and silence others

export default ({
  input: '...',
  warnings: {
    // valid values could be
    //   'error': throw an error instead of a warning
    //   true: keep this as a warning (the default, could also be 'warning')
    //   false: silence this warning
    MISSING_EXPORT: 'error', 
    CIRCULAR_DEPENDENCY: false
  },
  output: { /* ... */ }
});

Elevate ALL warnings to errors with some exceptions

export default ({
  input: '...',
  warnings: {
    all: 'error',
    MISSING_EXPORT: true, 
    CIRCULAR_DEPENDENCY: false
  },
  output: { /* ... */ }
});

Elevate all warnings to errors with NO exceptions

export default ({
  input: '...',
  warnings: 'error',
  output: { /* ... */ }
});

An open question would be how people would learn about the correct error codes. At the moment, the only sure way is to add an onwarn handler and log the codes. Maybe we can sneak the codes into the nice error messages but in a non-dominant way.

[lukastaegert]

#2981 will fix the behaviour of onwarn when --silent is used.

[shellscape]

#2981 was merged