nfour / serverless-build-plugin

A Node.js focused build plugin for serverless.
MIT License
41 stars 9 forks source link

Feature Request: Rename .jsx to .js when using babelify #23

Closed arabold closed 7 years ago

arabold commented 7 years ago

For packaging I'm using babelify but not uglify as I prefer having separate files. It keeps the source more readable and shouldn't matter on the server too much anyway.

My project is based on React and uses .jsx as preferred file extension instead of .js. Dependent code such as myClass.jsx is still loaded via require('myClass') without the need to specify any extension. This works locally as this is handled transparently via Babel. However, after deploying to my backend, Babel isn't available anymore and thus require('myClass') will fail as the default Node environment wouldn't look for any .jsx files. A simple way to fix this would be to rename all .jsx to .js during packaging (if Babel if enabled).

For the time being I'm using this as a workaround:

require.extensions['.jsx'] = require.extensions['.js'];

require('myClass'); // now is able to load *.jsx, too

Would such a feature make sense for the build plugin in your opinion? How else would you approach this problem?

nfour commented 7 years ago

See feature/normalizeBabelExt

Let me know if that fixes it when you add the option normalizeBabelExt: true to your root config.

arabold commented 7 years ago

Thanks for the update! Seems to work fine. However, I think there might also be a bug in the implementation:

+      result = {
+        ...result,
+        ...transformed,
+        relPath: this.options.normalizeBabelExt
+          ? relPath.replace(/\.[^.]+$/, '.js')
+          : relPath,
+      };

This replaces any file extension with .js, doesn't it? Shouldn't non-js files (e.g. .json and other configs) be kept untouched?

Though the new feature generally works, I'm not sure if my initial request really made that much sense in first place. This is such a specific case that I'm worried that adding such features overcomplicates the builder plugin over time and causes more harm. Especially as there are alternative options to solve it: Either use require.extensions as I do above, use .js instead of .jsx, or probably some other solutions as well. It just doesn't feel right to have that flag... I personally suggest to remove it again. Sorry for the back and forth.

nfour commented 7 years ago

That babel parser only runs over .js/jsx files so it's safe if I recall correctly.

The flag is actually valid, as down the road we may open up extensions as an option, for the edge cases. When we webpack or browserify we also manually key in the .js output filename. Because we're doing it on a per-file basis it seems logical.

It could be down the road it would be best to simply make options.normalizeBabelExt an array of tuples to map extensions.

For now, there really is only two widespread file extensions related to babel, and this covers that :). I appreciate your concern about bloat though, it's good to have a level head.