Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sapper server code minification #3

Closed
tommywalkie opened this issue Sep 9, 2020 · 2 comments
Closed

Sapper server code minification #3

tommywalkie opened this issue Sep 9, 2020 · 2 comments

Comments

@tommywalkie
Copy link
Owner

Back in Sapper 0.28.0 (#9eb4d43), rollup-plugin-esbuild was able to bundle and minify Sapper server code without issues.
When upgrading to Sapper 0.28.1 (#9332cfc), the following code is added on any generated Sapper server module (src/node_modules/@sapper/server.mjs) :

function createCommonjsModule(fn, basedir, module) {
   return module = {
       path: basedir,
       exports: {},
       require: function (path, base) {
           return commonjsRequire(path, (base === undefined || base === null) ? module.path : base);
       }
   }, fn(module, module.exports), module.exports;
}

Which is fine, but when rollup-plugin-esbuild tries to minify it, it is adding ES2020 nullish coalescing (a ?? b) in commonjsRequire(...) second parameter which leads to SyntaxError: Unexpected token '?' when starting the server.

function createCommonjsModule(e, t, r) {
    return r = {
        path: t,
        exports: {},
        require: function (n, o) {
            return commonjsRequire(n, o ?? r.path)
        }
    }, e(r, r.exports), r.exports
}

As a temporary workaround, the Rollup config will minify or not the Sapper server module based on the Sapper version.

@mrazauskas
Copy link

Is there a need to minify server side code? Take a look at the official Sapper template. terser() is not included in server: part. Only client: and serviceworker: are minified by Terser, because only these will be send over internet.

Seems like Node is supporting the nullish coalescing operator from version 14. Setting esbuild({ target: 'node12' }) fixes the issue you have (should work with node13 too, but I picked LTS version). The line looks like this after minification:

return commonjsRequire(o,r==null?n.path:r)

By the way, I was surprised to find out that target in esbuild options is not the same as target in tsconfig file. Check the docs.

@tommywalkie
Copy link
Owner Author

Is there a need to minify server side code? Take a look at the official Sapper template. terser() is not included in server: part

Oh, that makes sense. @mrazauskas Thanks for the clue.

By the way, I was surprised to find out that target in esbuild options is not the same as target in tsconfig file. Check the docs.

Mmmmm, unless I misunderstood the statement; this is incorrect. Here is the target ESBuild option value :

And this is the target field value in @tsconfig/svelte which is extending our tsconfig file.

{
  "$schema": "https://json.schemastore.org/tsconfig",
  "display": "Svelte",

  "compilerOptions": {
    ...
+   "target": "es2017",
    ...
  }
}

FWIW, this template was mostly designed a while ago when ESBuild was only at 0.6.x, before the new docs came out, and before the recent 0.8.x breaking changes. There sure may be ways to improve the current setup.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants