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

Resolution causes cjs file to be imported even though js file exists #395

Open
huntercaron opened this issue Dec 10, 2024 · 1 comment
Open

Comments

@huntercaron
Copy link

I am not sure if this is to be expected, but I am seeing this behaviour in the following reproduction.

Behaviour

  • https://ga.jspm.io/npm:[email protected]/dist/chart.cjs (see .cjs extension) is added to the scopes
  • This code doesn't run properly
  • It seems to be due to the resolution

Expected

import { Generator } from "@jspm/generator"

const generator = new Generator({
  mapUrl: "about:blank",
  inputMap: undefined,

  env: ["production", "browser", "module"],
  resolutions: { "chart.js": "^4.4.0" },
  defaultProvider: "jspm.io",
  fetchRetries: 10,
})

await generator.install([
  "react-chartjs-2",
  "chart.js",
  "chartjs-plugin-colorschemes",
])

console.log(generator.getMap())
{
  imports: {
    'chart.js': 'https://ga.jspm.io/npm:[email protected]/dist/chart.js',
    'chartjs-plugin-colorschemes': 'https://ga.jspm.io/npm:[email protected]/dist/chartjs-plugin-colorschemes.js',
    'react-chartjs-2': 'https://ga.jspm.io/npm:[email protected]/dist/index.js'
  },
  scopes: {
    'https://ga.jspm.io/': {
      '@kurkle/color': 'https://ga.jspm.io/npm:@kurkle/[email protected]/dist/color.esm.js',
      'chart.js': 'https://ga.jspm.io/npm:[email protected]/dist/chart.cjs',
      react: 'https://ga.jspm.io/npm:[email protected]/index.js'
    },
    'https://ga.jspm.io/npm:[email protected]/dist/chart.cjs': {
      '@kurkle/color': 'https://ga.jspm.io/npm:@kurkle/[email protected]/dist/color.cjs'
    }
  }
}
@guybedford
Copy link
Member

So this is correct resolution behaviour unfortunately - chart.js defines its package.json as:

  "exports": {
    ".": {
      "types": "./dist/types.d.ts",
      "import": "./dist/chart.js",
      "require": "./dist/chart.cjs"
    },
    "./auto": {
      "types": "./auto/auto.d.ts",
      "import": "./auto/auto.js",
      "require": "./auto/auto.cjs"
    },
    "./helpers": {
      "types": "./helpers/helpers.d.ts",
      "import": "./helpers/helpers.js",
      "require": "./helpers/helpers.cjs"
    }
  },

which means that when a CommonJS require loads chart.js, it will get dist/chart.cjs whereas an ES module willl get dist/chart.js instead.

That is, chart.js is badly defined and defines different versions for ESM and CJS and this is a bug in the package itself.

As for the interop issue - the chart.cjs module is actually correct, it's reading .defaults.global.plugins but in the latest version of ChartJS this is .defaults.plugins that is the global property of the defaults object no longer exists in the latest version. So the plugin is not compatible with this newer version of ChartJS as it needs to be updated to reference the .defaults.plugins object instead. This is not a JSPM interop issue either.

So both issues are package issues here unfortunately, and not JSPM issues.

For the first I would suggest to PR chart.js to avoid the dual package hazard by defining one main module.

And for the second I would suggest to use an updated version of the plugin that references .defaults.plugins.

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