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

Make browser usage more clear/compatible #653

Closed
kewisch opened this issue Apr 1, 2024 · 8 comments · Fixed by #659
Closed

Make browser usage more clear/compatible #653

kewisch opened this issue Apr 1, 2024 · 8 comments · Fixed by #659
Labels

Comments

@kewisch
Copy link
Owner

kewisch commented Apr 1, 2024

There are some breaking changes w.r.t. imports and browsers in 2.0.0. You can't simply <script src> import it there, you'll need

<script type="module">
import ICAL from "https://unpkg.com/ical.js";

// ...
</script>

It isn't currently possible to simply global import ICAL, even with the es5 version. Tasks here:

  1. Update README with a section on browser use
  2. Update es5 version to work like before where there is an ICAL global object.
@kewisch kewisch added the bug label Apr 1, 2024
@kewisch
Copy link
Owner Author

kewisch commented Apr 7, 2024

I looked into this a bit and would appreciate some help. Based on the rollup config, the cjs version adds module.exports = ICALmodule; to the end, which will fail when included in the browser. I can't switch to a custom IIFE version because of an error message where wrapping might break.

How do I get a joint es5 version that will work both as a script tag inclusion with ICAL as a global symbol, and as a cjs node module?

@mschroeder
Copy link
Collaborator

What about using good old UMD? I may be wrong, because I just checked if rollup runs with the config and produces something that looks acceptable:

{
  file: 'dist/ical.umd.js',
  exports: "default",
  name: 'ICAL',
  format: 'umd'
}

@kewisch
Copy link
Owner Author

kewisch commented Apr 8, 2024

Ah yes UMD might work quite well, thank you. I tried a bit today, but I can't get this to work in combination with babel. My goal is to have an es5 compatible version that doesn't use anything fancy.

[!] (plugin babel) Error: Using Babel on the generated chunks is strongly discouraged for formats other than "esm" or "cjs" as it can easily break wrapper code and lead to accidentally created global variables. Instead, you should set "output.format" to "esm" and use Babel to transform to another format, e.g. by adding "presets: [['@babel/env', { modules: 'umd' }]]" to your Babel options. If you still want to proceed, add "allowAllFormats: true" to your plugin options. 

Which gives me

[!] (plugin babel) Error: [BABEL] unknown file: Using removed Babel 5 option: .modules - Use the corresponding module transform plugin in the `plugins` option. Check out http://babeljs.io/docs/plugins/#modules

From there I tried a few other things, but couldn't get it to dial back its features.

@mschroeder
Copy link
Collaborator

How about using Babel on the input and not as an output plugin then, which should stop the error from happening:

{
  file: 'dist/ical.umd.js',
  exports: "default",
  name: 'ICAL',
  format: 'umd',
  plugins: [
    babel({ babelHelpers: 'bundled', presets: ['@babel/preset-env'] })
  ]
}

@kewisch
Copy link
Owner Author

kewisch commented Apr 8, 2024

Ah this gave me the right hints. I tried this before, but the plugin location was wrong, so it tried to load babel as an output plugin even though it was not. Here is a config that seems to work:

import { babel } from '@rollup/plugin-babel';
import { terser } from 'rollup-plugin-terser';

export default [{
  input: 'lib/ical/module.js',
  output: [
    { file: 'dist/ical.js',  format: 'es', exports: 'default' },
    {
      file: 'dist/ical.min.js',
      format: 'es',
      exports: 'default',
      plugins: [terser()]
    }
  ]
},{
  input: 'lib/ical/module.js',
  output: [
    {
      file: 'dist/ical.es5.cjs',
      exports: 'default',
      name: 'ICAL',
      format: 'umd',
    },
    {
      file: 'dist/ical.es5.min.cjs',
      exports: 'default',
      name: 'ICAL',
      format: 'umd',
      plugins: [terser()]
    }
  ],
  plugins: [
    babel({ babelHelpers: 'bundled', presets: ['@babel/preset-env'] })
  ]
}];

Does that look right?

@mschroeder
Copy link
Collaborator

Does that look right?

It does to me, and the output is similar to what it was before, despite the added wrapper function, how the babel helpers are included and stripped license comments.

@kewisch
Copy link
Owner Author

kewisch commented Apr 8, 2024

Thanks! I'll look into the missing license header for this one and check the others. We should have at least one at the top.

@mschroeder
Copy link
Collaborator

There is at least one right before the included module.js near the end of the generated file. 😉

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

Successfully merging a pull request may close this issue.

2 participants