hackerwins / codepair-old

Real-time markdown editor for interviews, meetings and more...
https://codepair.yorkie.dev
Apache License 2.0
90 stars 29 forks source link

Use sass (dart-sass) instead of node-sass #196

Closed blurfx closed 2 years ago

blurfx commented 2 years ago

Description: Use sass(dart-sass) instead of node-sass.

Why:

gollumnima commented 2 years ago

@blurfx Could you assign this issue to me? I'll install sass and update any other libraries like typescript and eslint

gollumnima commented 2 years ago

After updating typescript version, I found type errors in src/components/Editor/DrawingBoard/Canvas/dom.ts file. The problem is Property 'pointerEnabled' and 'msPointerEnabled' don't exist on type 'Navigator'.

/**
 * {@linkcode https://github.com/bevacqua/dragula/blob/e0bcdc72ae8e0b85e17f154957bdd0cc2e2e35db/dragula.js#L498}
 */

export function touchy(el: EventTarget, event: Function, type: MouseType, fn: (evt: TouchyEvent) => void) {
  if (global.navigator.pointerEnabled) { // 🚨🚨🚨🚨🚨🚨🚨
    event(el, pointers[type], fn);
  } else if (global.navigator.msPointerEnabled) { // 🚨🚨🚨🚨🚨🚨🚨
    event(el, microsoft[type], fn);
  } else {
    event(el, touch[type], fn);
    event(el, type, fn);
  }
}

So I've looked into dragula project and found dragula.min.js file. There was something in the file that seemed to declare a navigator function.

function U(e, t, n, o) {
  r.navigator.pointerEnabled
    ? k[t](
        e,
        {
          mouseup: 'pointerup',
          mousedown: 'pointerdown',
          mousemove: 'pointermove',
        }[n],
        o,
      )
    : r.navigator.msPointerEnabled
    ? k[t](
        e,
        {
          mouseup: 'MSPointerUp',
          mousedown: 'MSPointerDown',
          mousemove: 'MSPointerMove',
        }[n],
        o,
      )
    : (k[t](
        e,
        {
          mouseup: 'touchend',
          mousedown: 'touchstart',
          mousemove: 'touchmove',
        }[n],
        o,
      ),
      k[t](e, n, o));
}

Like this, I think we need to declare our own navigator type in the project. Or is there any other good choice to fix this issue? 🥲 Or do I roll back typescript to previous version for the backward compatibility?

ppeeou commented 2 years ago

@gollumnima It seems that the dom type has changed a lot since typescript 4.4. https://github.com/microsoft/TypeScript-DOM-lib-generator/issues/1029 pointerEnabled, msPointerEnabled is used to support older environments (Window)

Initially, we wrote the code to support old browsers Now, ie no longer needs to be supported, so I think we can delete the code. Also, we have already stated in the package.json browserlist that only the latest browser environment is supported.

ppeeou commented 2 years ago

If we delete the code, I think it's better to split the pr. Even if we use 'any' type

blurfx commented 2 years ago

note: please use unknown instead of any if possible :)

metleeha commented 2 years ago

@blurfx Sorry for being late. I was running out of time, so I requested a pr without permission. I simply modified package.json and uploaded it.