Is it necessary to transform (via babel) and minify packaged code?

[askoufis]

I've noticed that this package runs babel over its packaged code, as well as minifying it with terser.

didyoumean2/rollup.config.ts

Lines 36 to 46 in 14d55cb

	babel({
	babelHelpers: 'runtime',
	extensions: ['.js', '.ts'],
	}),
	commonjs({
	include: '**/node_modules/**',
	}),
	resolve({
	extensions: ['.js', '.ts'],
	}),
	terser(),

I think both of these steps are unnecessary and should be removed from the build pipeline for 2 reasons:

1. Generally it's on the consumer to perform these steps if desired, and I don't think it's good practice to force consumers to consume minified and/or transformed code as it can make debugging harder (though this can be somewhat alleviated with the help of sourcemaps, which this package does provide).
2. In the case of this package, there's an extra dependency, specifically @babel/runtime, in package.json. Again, if a consumer wants to utilize this package in combination with @babel/plugin-transform-runtime, then that should be up to them. Furthermore, @babel/runtime doesn't actually end up used in the final packaged files, so it's not even really required for this package.

Happy to discuss this further if necessary, though I don't think this is too controversial of an idea. If it's agreed that this change is worthwhile, then I'd be happy to contribute the necessary changes.

[foray1010]

@askoufis Thanks a lot for the detailed feedbacks! I agree with your points on minifier. The downside though is it increases the cost to import this library via modern javascript module, e.g. import didYouMean from "https://example.com/didyoumean2.js"; and <script type="module">. But I rarely see people doing it in production, so probably the pros outweight the cons. Feel free to open PR for that!

For @babel/plugin-transform-runtime, compared to inline babel helpers, I think it is useful for frontend as bundler can deduplicate babel polyfills and reduce the bundle size. This cannot be achieved afterwards if we use inline babel helpers. We can stop transforming the code and let user configure their bundler to achieve the same effect. While it works on FE, it does not work with nodejs because people rarely transpile dependencies on backend.

I am awared that we are not using any features that is outside of targeted nodejs version range ^18.12.0 || >=20.9.0, so @babel/runtime is not imported. But if we ever use features that is not in the range, that could cause an issue. A middle ground could be we use eslint (n/no-unsupported-features/es-builtins and n/no-unsupported-features/es-syntax) to enforce this never happens. I am fine with that since modern js is mostly pleasant to read and write anyway.

[askoufis]

I hadn't considered the case where you're loading this package via a script element. However, currently this wouldn't work out of the box because the bundled file still contains imports for its dependencies, so the user would need to use import maps to resolve those. You could bundle this package's dependencies if you really wanted to cater to that use case.

I'd be happy to keep the minified bundle, but it could instead be distributed alongside the regular, non-minified files, and consumers could include it in their HTML via a CDN like unpkg.

[jdalton]

I came here for this too. You could provide a .min.js variation. Inspecting the source at a glance on npm is handy to know if a package is malicious. Seeing it minified raised flags.

[foray1010]

Inspecting the source at a glance on npm is handy to know if a package is malicious. Seeing it minified raised flags.

It makes sense, I didn't consider this before.

You could provide a .min.js variation

currently this wouldn't work out of the box because the bundled file still contains imports for its dependencies, so the user would need to use import maps to resolve those.

All valid, I may do that when js module is requested in the future.

Considering all that, I have removed the minification on version 7.0.4. Thank you so much for the feedbacks!

[jdalton]

Thank you @foray1010!