thisbeyond / solid-dnd

A lightweight, performant, extensible drag and drop toolkit for Solid JS.
https://solid-dnd.com
MIT License
516 stars 34 forks source link

console.warn in production #79

Closed thetarnav closed 1 year ago

thetarnav commented 1 year ago

There are a lot of warnings emitted from drag-drop-context.tsx which should probably be removed in production build as they are not useful for end users and bloat the bundle.

I've recently made tsup-preset-solid to help with that so this should be an easy fix

wanna PR? :)

martinpengellyphillips commented 1 year ago

they are not useful for end users

Interesting - I deliberately had those warnings in there to help consumers using the library figure out when their setup wasn't working. Otherwise I would get rid of them completely (I don't need them for development 😄).

I wonder what the best approach here is. Can consumers get rid of them when building their production build? Is that something your preset solves?

thetarnav commented 1 year ago

they are not useful for end users

by end users I actually meant ..um "actual users" (non-devs, in production) :D

Can consumers get rid of them when building their production build?

The preset does what solid-js does, there are two different entries shipped, one for development and one for production. So you can have as many warnings in development as neccesary, or perform complarely different logic depending on the env.

This is achieved by utilizing export conditions, e.g.

// package.json
{
  "exports": {
    "developmnet": "./dist/dev.js",
    "default": "./dist/index.js"
  }
}

and it works very well, maybe besides solid-start now as there is an issue with module resolution which causes the dev version to be loaded in production as well

thetarnav commented 1 year ago

The entries are created from the same source so there is no dupplication. In solid-primitives we make it explicit by doing if (process.env.DEV) console.log(...) but there is an option to just remove all console.logs from prod.

martinpengellyphillips commented 1 year ago

Ah, okay - sounds like a win then. Please do a PR 😄

This might be answered in the PR, but I'm curious how this affects the current setup I have? E.g. does it mean I can get rid of the following in package.json and it will just work?

 "exports": {
    ".": {
      "solid": "./dist/source/index.jsx",
      "default": "./dist/esm/index.js"
    }
  },