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

'?' navigation operator #1972

Closed
felipedrumond opened this issue Jan 8, 2019 · 33 comments · Fixed by #5013
Closed

'?' navigation operator #1972

felipedrumond opened this issue Jan 8, 2019 · 33 comments · Fixed by #5013

Comments

@felipedrumond
Copy link

Is there any navigation operator for html rendering properties?

{ bookingFlow.nextStep.question } throws an exception when 'nextStep' is undefined. That exception could be bypassed using a navigation operator '?' { bookingFlow.nextStep**?**.question }, therefore making a expression like this possible:

// if nextStep is neither undefined nor null, displays its question property value, otherwise, displays the startingQuestion
{ bookingFlow.nextStep?.question || bookingFlow.startingQuestion }

@chris-morgan
Copy link
Contributor

There’s a Stage 1 proposal for this in JavaScript: https://github.com/tc39/proposal-optional-chaining.

@Rich-Harris
Copy link
Member

I don't think Svelte should be in the business of adding non-standard JavaScript inside expressions — there are plenty of ways to deal with this scenario. We could maybe consider whether we want to expose a way to allow things like Babel to manipulate JS expressions inside the template, but it shouldn't be built in to Svelte until it's a part of JavaScript, so I'll close this.

@xamgore
Copy link

xamgore commented Nov 10, 2019

Optional chaining proposal is at stage 3 now. Also landed in Typescript 3.7.

@snaerth
Copy link

snaerth commented Feb 8, 2020

Stage 4 now and in Chrome 80 as well. It is coming!

@ioannist
Copy link

standard or not yet standard, it is super handy :D +1 for null propagation operator

@Conduitry
Copy link
Member

The last I checked, Acorn did not support parsing this, because the ESTree AST wasn't final yet, which would be a prerequisite for working on this.

@deadit
Copy link

deadit commented Mar 26, 2020

What about now?

@Conduitry
Copy link
Member

No. The PRs to watch are acornjs/acorn#890 and acornjs/acorn#891

@Conduitry Conduitry reopened this Mar 26, 2020
@adamduncan
Copy link

Looks like nullish coalescing is merged. 🤞 on the optional chaining...

Related: #4701

@russelgal
Copy link

Finally:
Add support for optional chaining (?.).
https://github.com/acornjs/acorn/releases/tag/7.3.0

@Conduitry
Copy link
Member

Oh, awesome! That's exciting

@frederikhors
Copy link

@Conduitry how can I help with this?

@avimar
Copy link

avimar commented Jun 27, 2020

Assuming this merged update means this will now work with the compiler?

Will this be available within the template too, e.g. {#if object?.property?.result}

Are we expecting a new release soon?

Thanks!

@Conduitry
Copy link
Member

This is now supported in 3.24.0. It works in templates as well as the <script> tag, yes.

@avimar
Copy link

avimar commented Jul 7, 2020

https://svelte.dev/repl/7b67c23b82e84cd9bdc634fea392d025?version=3.24.0

I'm getting a warning on the bottom name is not defined when using name?.test

Is this just a warning?

@j3rem1e
Copy link

j3rem1e commented Jul 7, 2020

In your repl, the variable is spelled "names", not "name"

@avimar
Copy link

avimar commented Jul 7, 2020

Exactly, am I misunderstanding? I thought it can check if the top level variable is not existent too.

@trbrc
Copy link
Contributor

trbrc commented Jul 7, 2020

It can check if it’s null or undefined, but not if it isn’t even declared. You can only check that with typeof name.

@voscausa
Copy link

3.24.0 optional chaning works fine for me in dev mode. But when I try to build I receive:

if (accounts.map[a_id]?.dirtyYears?.includes(year)) {
[!] (plugin terser) SyntaxError: Unexpected token: punc (.)

@tanhauhau
Copy link
Member

@voscausa Optional chaining is not yet supported by Terser, terser/terser#567

@mvolkmann
Copy link

Shouldn't this issue have been left open until optional chaining and nullish coalescing work when "npm run build" is run? Having them only with with "npm run dev" seems like it's only good for experimenting.

@trbrc
Copy link
Contributor

trbrc commented Jul 26, 2020

@mvolkmann Whether npm run build uses Terser depends on your configuration, and is outside of Svelte's control, at least for this repo. It might make sense to bring up in sveltejs/template.

@mvolkmann
Copy link

I wasn't viewing sveltejs/template as separate from Svelte because I thought that's mostly the recommended way to start new Svelte projects. But I will create an issue there if it doesn't already exist.

@avimar
Copy link

avimar commented Jul 27, 2020

Originally posted by @benmccann in sveltejs/template#134 (comment)

Here's the terser issue to track for reference: terser/terser#567

@shirakaba
Copy link

Am I right in understanding that code using optional chaining would still be able to build with Terser as long as you used svelte-preprocess to make Babel transpile the optional-chaining syntax to less modern expressions that Terser would be more likely to support?

@avimar
Copy link

avimar commented Jul 29, 2020

Am I right in understanding that code using optional chaining would still be able to build with Terser as long as you used svelte-preprocess to make Babel transpile the optional-chaining syntax to less modern expressions that Terser would be more likely to support?

Just based on playing with rollup in the past, you would need a svelte specific bablel config. You can simply transpire before it gets to the terser.

@antony
Copy link
Member

antony commented Jul 29, 2020

It's easier to discuss usage on discord - but you don't use babel inside Svelte preprocess, you just add it to your rollup configuration, and this is the way to get it working with terser.

@mvolkmann
Copy link

@antony Is it correct to say that if we really want to use optional chaining now, there is a way to configure Rollup so that terser will work, but we expect this to be fixed in terser soon so that we don't have to configure Rollup in order for it to work?

@shirakaba
Copy link

@avimar I'm not sure what you mean differently by setting up a "Svelte-specific babel config". I think that was what I was suggesting.

you don't use babel inside Svelte preprocess, you just add it to your rollup configuration, and this is the way to get it working with terser.

@antony I'm not sure what you mean differently, again – the Rollup config can define the Babel config to be passed into Svelte Preprocess, and that's exactly what's recommended in the context of supporting optional chaining and nullish coalescing in the docs for svelte-preprocess that I linked:

import preprocess from 'svelte-preprocess'
  ...
  preprocess: preprocess({
    babel: {
      presets: [
        [
          '@babel/preset-env',
          {
            loose: true,
            // No need for babel to resolve modules
            modules: false,
            targets: {
              // ! Very important. Target es6+
              esmodules: true,
            },
          },
        ],
      ],
    },
  });
  ...

(Not to say that this is exactly the Babel config you'd need, but that's how the docs describe to pass a Babel config to Svelte Preprocess).

@shirakaba
Copy link

shirakaba commented Jul 29, 2020

I have this working now.

  1. Ensure you have Svelte version 3.24.0 or higher, as that supports Optional Chaining and Nullish Coalescing without requiring any svelte-preprocess step. That is to say, this version of the Svelte compiler understands those features directly.
  2. Introduce a babel transpilation step (i.e. in your Rollup config's plugins array) before your code is read by Terser.

Here's my exact config:

rollup.config.js

import svelte from 'rollup-plugin-svelte';
import resolve from '@rollup/plugin-node-resolve';
import commonjs from '@rollup/plugin-commonjs';
import livereload from 'rollup-plugin-livereload';
import { terser } from 'rollup-plugin-terser';
import babel from 'rollup-plugin-babel';

const production = !process.env.ROLLUP_WATCH;

export default {
  input: 'src/main.js',
  output: {
    sourcemap: !production,
    format: 'iife',
    name: 'app',
    file: 'public/build/bundle.js'
  },
  plugins: [
    svelte({
      // enable run-time checks when not in production
      dev: !production,
      // we'll extract any component CSS out into
      // a separate file - better for performance
      css: css => {
        css.write('public/build/bundle.css');
      }
    }),

    // If you have external dependencies installed from
    // npm, you'll most likely need these plugins. In
    // some cases you'll need additional configuration -
    // consult the documentation for details:
    // https://github.com/rollup/plugins/tree/master/packages/commonjs
    resolve({
      browser: true,
      dedupe: ['svelte']
    }),
    babel({
      extensions: ['.js', '.mjs', '.html', '.svelte'],
      include: ['src/**', 'node_modules/svelte/**'],
    }),
    commonjs(),

    // In dev mode, call `npm run start` once
    // the bundle has been generated
    !production && serve(),

    // Watch the `public` directory and refresh the
    // browser on changes when not in production
    !production && livereload('public'),

    // If we're building for production (npm run build
    // instead of npm run dev), minify
    production && terser()
  ],
  watch: {
    clearScreen: false
  }
};

function serve() {
  let started = false;

  return {
    writeBundle() {
      if (!started) {
        started = true;

        require('child_process').spawn('npm', ['run', 'start', '--', '--dev'], {
          stdio: ['ignore', 'inherit', 'inherit'],
          shell: true
        });
      }
    }
  };
}

babel.config.js

module.exports = {
  presets: [
    [
      '@babel/preset-env',
      {
        targets: {
          firefox: "51",
          safari: "602.2.14",
        },
        corejs: '3.6',
        useBuiltIns: 'usage',
        shippedProposals: false,
      },
    ],
  ],
};

I also have the following dependencies to support the Babel transpilation:

{
  "name": "some-svelte-app",
  "version": "1.0.0",
  "scripts": {},
  "devDependencies": {
    "@babel/core": "^7.10.2",
    "@babel/preset-env": "^7.10.2",
    "@rollup/plugin-commonjs": "^11.1.0",
    "@rollup/plugin-node-resolve": "^7.1.3",
    "rollup": "^1.32.1",
    "rollup-plugin-babel": "^4.4.0",
    "rollup-plugin-livereload": "^1.3.0",
    "rollup-plugin-svelte": "^5.2.2",
    "rollup-plugin-terser": "^5.3.0",
    "svelte": "^3.24.0"
  },
  "dependencies": {
    "core-js": "^3.6.5",
    "sirv-cli": "^0.4.6"
  }
}

Note: I'm a novice with Core JS and at setting up Babel so there may be some redundancy in there, but anyway, this config does work (and indeed fails on the Terser step if you omit the babel() step).

Results:

// Original code in a .svelte file
const whatever = document.getElementById("sdgh")?.style?.height ?? "123px";
console.log(`whatever returned ${whatever}`);

// Code emitted after Babel transpilation
const whatever = (_document$getElementB = (_document$getElementB2 = document.getElementById("sdgh")) === null || _document$getElementB2 === void 0 ? void 0 : (_document$getElementB3 = _document$getElementB2.style) === null || _document$getElementB3 === void 0 ? void 0 : _document$getElementB3.height) !== null && _document$getElementB !== void 0 ? _document$getElementB : "123px";
console.log(`whatever returned ${whatever}`);

// Code emitted after Terser minification of the Babel output from above
m=null!==(i=null===(a=document.getElementById("sdgh"))||void 0===a||null===(s=a.style)||void 0===s?void 0:s.height)&&void 0!==i?i:"123px";return console.log("whatever returned "+m)

@benmccann
Copy link
Member

This is now supported by terser. I filed a request to add it to rollup-plugin-terser: TrySound/rollup-plugin-terser#85

@TrySound
Copy link

See TrySound/rollup-plugin-terser#84 (comment)

@benmccann
Copy link
Member

Thanks for the clarification! I've sent a PR to upgrade the svelte template so that new users get this by default: sveltejs/template#164

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

Successfully merging a pull request may close this issue.