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

tinycolor is not a function #249

Closed
Babyfaceqian opened this issue Dec 22, 2022 · 39 comments · Fixed by #259
Closed

tinycolor is not a function #249

Babyfaceqian opened this issue Dec 22, 2022 · 39 comments · Fixed by #259

Comments

@Babyfaceqian
Copy link

I just upgraded tinycolor@1.5.0 and code like this:

const tinycolor = require('tinycolor2');
tinycolor(color)

It comes tinycolor is not a function error, plz take a look.

@bgrins
Copy link
Owner

bgrins commented Dec 22, 2022

Oh no, I had tested this in #245 but apparently not well enough. I will roll this back in a 1.5.1 and we can try to debug. Can you share any more details or a STR?

@bgrins
Copy link
Owner

bgrins commented Dec 22, 2022

I'll deploy a new version out of 1.4.2-branch

@bgrins
Copy link
Owner

bgrins commented Dec 22, 2022

@Babyfaceqian I just published 1.5.1 which should match 1.4.2

bgrins added a commit that referenced this issue Dec 22, 2022
@bgrins
Copy link
Owner

bgrins commented Dec 22, 2022

Can you confirm it's working again?

@bgrins
Copy link
Owner

bgrins commented Dec 22, 2022

Also, which version of node are you using?

@Babyfaceqian
Copy link
Author

Also, which version of node are you using?

1.5.1 works.
The node version is v14.20.1.

@bgrins
Copy link
Owner

bgrins commented Dec 22, 2022

@Babyfaceqian I set this test case up on https://runkit.com/embed/kzh7l5l3wocg and it seems to work. Any chance you can send me a test case / repo where this is failing?

@bgrins
Copy link
Owner

bgrins commented Dec 22, 2022

@Babyfaceqian I published a beta version as 1.6.0, can you please check that? https://www.npmjs.com/package/tinycolor2?activeTab=versions. It can be installed with npm install tinycolor2@beta

@bgrins
Copy link
Owner

bgrins commented Dec 23, 2022

Checked on runkit with the beta, going back to Node 4.0: https://runkit.com/embed/fdd1jg4lfegd

var tinycolor2 = require("tinycolor2@1.6.0-beta.2")
console.log(tinycolor2("red").toString());

Returns red as expected

@bgrins bgrins mentioned this issue Dec 23, 2022
3 tasks
@Babyfaceqian
Copy link
Author

@bgrins Sorry for the late response.
I was using tinycolor2 in browser side with commonjs syntax, it works before version 1.5.0. When upgraded to 1.5.0, it works with esmodule syntax, but doesn't work with commonjs syntax, that's why I got "tinycolor is not a function" error.

@bgrins
Copy link
Owner

bgrins commented Dec 23, 2022

Were you loading it with a CDN like https://cdn.jsdelivr.net/npm/tinycolor2/ or https://cdnjs.cloudflare.com/ajax/libs/tinycolor/, or directly from npm? And if from npm can you share what scripts you were using to build for the browser?

It's possible tinycolor2@1.6.0-beta.2 will fix it since I've moved tinycolor.js and dist/tinycolor-min.js into the npm directory.

@Babyfaceqian
Copy link
Author

let me try tinycolor2@1.6.0-beta.2 first

@Babyfaceqian
Copy link
Author

@bgrins tinycolor2@1.6.0-beta.2 doesn't works.
And I am loading it with npm. I am using npm start script and see the error in browser when it's loaded.

// package.json
{
  "name": "@cy/bi",
  "displayName": "tools",
  "version": "1.0.2",
  "description": "tools",
  "author": "zhangqian",
  "componentType": "3",
  "scripts": {
    "start": "dumi dev",
    "docs:build": "dumi build",
    "docs:deploy": "gh-pages -d docs-dist",
    "build": "father-build",
    "deploy": "npm run docs:build && npm run docs:deploy",
    "prettier": "prettier --write \"**/*.{js,jsx,tsx,ts,less,md,json}\"",
    "test": "umi-test",
    "test:coverage": "umi-test --coverage",
    "prepublishOnly": "npm run build"
  },
  "main": "lib/index.js",
  "module": "es/index.js",
  "typings": "lib/index.d.ts",
  "gitHooks": {
    "pre-commit": "lint-staged"
  },
  "lint-staged": {
    "*.{js,jsx,less,md,json}": [
      "prettier --write"
    ],
    "*.ts?(x)": [
      "prettier --parser=typescript --write"
    ]
  },
  "peerDependencies": {
    "react": ">=16.8.0",
    "js-cookie": "*",
    "uuid": "*"
  },
  "devDependencies": {
    "@types/jest": "^27.0.3",
    "@umijs/test": "^3.0.5",
    "antd": "^4.22.4",
    "babel-eslint": "^9.0.0",
    "babel-plugin-import": "^1.13.5",
    "babel-plugin-transform-remove-console": "^6.9.4",
    "dumi": "^1.1.0",
    "eslint": "^5.4.0",
    "eslint-config-ali": "^9.0.2",
    "eslint-plugin-import": "^2.26.0",
    "eslint-plugin-import-helpers": "^1.2.1",
    "eslint-plugin-jsx-a11y": "^6.5.1",
    "eslint-plugin-prettier": "^4.0.0",
    "eslint-plugin-react": "^7.29.4",
    "father-build": "^1.17.2",
    "gh-pages": "^3.0.0",
    "js-cookie": "^3.0.1",
    "lint-staged": "^10.0.7",
    "prettier": "^2.2.1",
    "uuid": "^8.3.2",
    "yorkie": "^2.0.0"
  },
  "dependencies": {
    "tinycolor2": "^1.6.0-beta.2"
  }
}

// docs/index.md
const tinycolor = require('tinycolor2');
console.log(tinycolor("red").toString())

@Babyfaceqian
Copy link
Author

@bgrins I saw below code in the package.json of tinycolor2@1.6.0-beta.2, the exports config doesn't cover browser runtime right?

// node_modules/tinycolor2/package.json
// ...
"exports": {
    ".": {
      "import": "./esm/tinycolor.js",
      "require": "./cjs/tinycolor.js"
    }
  },
// ...

@bgrins
Copy link
Owner

bgrins commented Dec 23, 2022

Thanks for looking into it. I didn't realize it had to specify a browser export specifically - tbh the config and docs for it are not very clear to me. Would I also need a sibling to

 "module": "./esm/tinycolor.js",
  "main": "./cjs/tinycolor.js",

or just the exports > . > browser?

@bgrins
Copy link
Owner

bgrins commented Dec 23, 2022

As per https://nodejs.org/api/packages.html#conditional-exports maybe we need a default named export instead of browser

@bgrins
Copy link
Owner

bgrins commented Dec 23, 2022

Alright, published a beta 3 with the default export section at https://www.npmjs.com/package/tinycolor2/v/1.6.0-beta.3?activeTab=explore. Can you try that with your setup?

@Babyfaceqian
Copy link
Author

const tinycolor2 = require('tinycolor2');
console.log(tinycolor2)

image
hmmm...this is what i got with beta 3, same as beta 2

@bgrins
Copy link
Owner

bgrins commented Dec 23, 2022

Alright, tried publishing a beta 4 with the "browser" field in package.json (https://www.npmjs.com/package/tinycolor2/v/1.6.0-beta.4?activeTab=explore)

@bgrins
Copy link
Owner

bgrins commented Dec 23, 2022

Though it seems like the build tool is picking up the module export somehow

@bgrins
Copy link
Owner

bgrins commented Dec 23, 2022

I also went ahead and published a beta 5 which changed the order of the exports to try putting require first https://www.npmjs.com/package/tinycolor2/v/1.6.0-beta.5?activeTab=explore

@bgrins
Copy link
Owner

bgrins commented Dec 23, 2022

Let me know if 4 or 5 fixes it

@Babyfaceqian
Copy link
Author

@bgrins 4 fail, trying 5

@Babyfaceqian
Copy link
Author

5 same as before

@Babyfaceqian
Copy link
Author

Let me try another build tool

@bgrins
Copy link
Owner

bgrins commented Dec 23, 2022

Yeah I was wondering if there was any way to get debugging output from the build tool. It seems to be either grabbing ./esm/tinycolor.js for some reason or grabbing the cjs file and then converting it into esm (despite you using require and your package.json not specifying a module)

@bgrins
Copy link
Owner

bgrins commented Dec 23, 2022

The fact that it was working before (when we weren't exporting ESM makes me think it's the former - grabbing ./esm/tinycolor.js for some reason). I guess you could debug that in your node_modules folder by replacing the content of esm/tinycolor.js with cjs/tinycolor.js and see if it works.

@bgrins
Copy link
Owner

bgrins commented Dec 23, 2022

I know it's a totally different setup, but here's it working in the browser with parcel & ESM https://codesandbox.io/s/determined-meadow-6tip3b?file=/src/index.js

@Babyfaceqian
Copy link
Author

alright, i will create demo and send you tomorrow to keep looking into. It's too late here:)

@Babyfaceqian
Copy link
Author

@bgrins Plz check this demo: https://codesandbox.io/s/umi-demo-x8cfqf?file=/src/pages/index.tsx
If the app does not start automatically on the first loading, try run npm start in the Terminal and refresh in the browser panel.

I also digged into module main browser exports in package.json and found the docs https://webpack.js.org/guides/package-exports/. As the docs says, "exports field is preferred over other package entry fields like main, module, browser or custom ones.", so the build tool should take exports config first if it exists.But what the demo shows seems the build tool ignores the exports config and directly take module config. I did not figure out why...

I also tried to changed the configs in package.json of tinycolor2, and found one way to fix this by adding "browser": "./cjs/tinycolor.cjs" in the package.json.The build tool will take browser config first.You may publish one more with browser config and test in the demo.

@bgrins
Copy link
Owner

bgrins commented Dec 24, 2022

@Babyfaceqian thanks for making that sandbox and digging into the problem, I can reproduce the problem there. I did try adding a browser field in 1.6.0-beta.6 but am still seeing the problem in https://codesandbox.io/s/umi-demo-forked-l5ukny?file=/package.json for some reason:

"module": "./esm/tinycolor.js",
  "main": "./cjs/tinycolor.js",
  "browser": "./cjs/tinycolor.js",
  "exports": {
    ".": {
      "import": "./esm/tinycolor.js",
      "require": "./cjs/tinycolor.js"
    }
  },

From https://www.npmjs.com/package/tinycolor2/v/1.6.0-beta.6?activeTab=explore

@bgrins
Copy link
Owner

bgrins commented Dec 24, 2022

I do notice that in index.tsx you have

import React from 'react';
import styles from './index.less';
const tinycolor2 = require('tinycolor2');

(mixing require and import). Is it possible the build tool is confused by this? It seems to work fine I change it to:

import React from 'react';
import styles from './index.less';
import tinycolor2 from 'tinycolor2';

@bgrins
Copy link
Owner

bgrins commented Dec 24, 2022

Although it seems to work alright with const React = require("react");

@bgrins
Copy link
Owner

bgrins commented Dec 24, 2022

Maybe it is working with beta6 after all and it just wasn't re-running in code sandbox? It appears to be working for me now. Lmk if beta6 is good for you when you have a chance. If so I can add that browser field in a 1.5.3.

@Babyfaceqian
Copy link
Author

@bgrins I also see it is working in your code sandbox, beta6 is good for me, you may add browser field in a 1.5.3.
let me know when you have a change to figure out why the build tool of umi framework ignores exports config.If I find something new about it, will make an update to you.
Thanks for handling this issue.

bgrins added a commit that referenced this issue Dec 28, 2022
… of a build tool using the ESM export, and does not seem to negatively affect other tools which ignore this in lieu of the exports section - see for example https://webpack.js.org/guides/package-exports/. Fixes #249
bgrins added a commit that referenced this issue Dec 28, 2022
… of a build tool using the ESM export, and does not seem to negatively affect other tools which ignore this in lieu of the exports section - see for example https://webpack.js.org/guides/package-exports/. Fixes #249 (#259)
@bgrins
Copy link
Owner

bgrins commented Dec 28, 2022

Alright, I added the field in the main branch, and have published a 1.6.0-beta.8 from it. I'll push an official new version out in the next day or two.

@bgrins
Copy link
Owner

bgrins commented Jan 2, 2023

@Babyfaceqian I've pushed out https://www.npmjs.com/package/tinycolor2/v/1.5.2, let me know if there are issues with that.

@anisabboud
Copy link

import tinycolor2 from 'tinycolor2';  // Works with v1.5.2 (updated syntax).

alanorth added a commit to ilri/OpenRXV that referenced this issue Jul 10, 2023
Need to change the import syntax for v1.6.0.

See: bgrins/TinyColor#249
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

Successfully merging a pull request may close this issue.

3 participants