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

remove dependency on fs, http #478

Closed
sousarka opened this issue Apr 12, 2023 · 12 comments
Closed

remove dependency on fs, http #478

sousarka opened this issue Apr 12, 2023 · 12 comments

Comments

@sousarka
Copy link

Hello,

First of all, thank you for the excellent library. Really appreciate this.

Is there any plan to decouple this from fs, http (node js specific libraries)? Since there are already helper methods like toString()/toBlob()/toJson(), wanted to understand the reason behind the dependency to nodejs specific modules in this.

The specific save/serve methods can be in the readme if needed.

Let me know your thoughts.

@sebbo2002
Copy link
Owner

sebbo2002 commented Apr 13, 2023

Hello, fs has not been removed yet to preserve backward compatibility. In browser environments fs can be set to null by your bundler. Does this already solve your problem? http is only used for typing except in the unit tests, here the code could be adapted accordingly with import type.

@sousarka
Copy link
Author

sousarka commented Apr 14, 2023

Hello,
The only issue with setting fs to null in browser environment is the tools that most people work with in order to develope react/angular apps like create-react-app or ng, where developers can get all the simplified solutions without thinking of the bundler etc. In most cases, just to use this one library, they'll either have to eject the bundler out or try some other overrides (e.g. craco for react projects). This option may not be preferable for most developers since they'll not be able to use these packaged helpers (cra or ng) to upgrade/add-feature etc from that point onwards.

Let me know what you think.

@sebbo2002
Copy link
Owner

For the next minor release it is already planned to reduce dependencies. Removing fs is definitely one of them. But since there are also some other planned changes for this next major release (e.g. #344) and I have other projects with higher priority at the moment, it didn't happen yet.

@qertis
Copy link

qertis commented Apr 24, 2023

For the next minor release it is already planned to reduce dependencies. Removing fs is definitely one of them. But since there are also some other planned changes for this next major release (e.g. #344) and I have other projects with higher priority at the moment, it didn't happen yet.

I also want to note that the proposed option with Webpack configuration does not work with Vite. Because of what, it is not possible to work in this collector. Instead, errors are thrown:

[vite] error while updating dependencies:
Error: Build failed with 3 errors:
node_modules/ical-generator/dist/index.js:59:13: ERROR: No matching export in "browser-external:fs" for import "writeFile"
node_modules/ical-generator/dist/index.js:59:28: ERROR: No matching export in "browser-external:fs" for import "writeFileSync"
node_modules/ical-generator/dist/index.js:59:47: ERROR: No matching export in "browser-external:fs" for import "promises"

@sebbo2002
Copy link
Owner

In such a case I recommend to create a fork of this library (without fs), to use another library, or to do the creation of the calendar file server-side. I don't have the resources to work on the minor release at the moment.

@qertis
Copy link

qertis commented Apr 27, 2023

In such a case I recommend to create a fork of this library (without fs), to use another library, or to do the creation of the calendar file server-side. I don't have the resources to work on the minor release at the moment.

I have already created a library with such functionality, and it does not require any dependencies. You can use this library instead, as it avoids the need for fs altogether.
https://www.npmjs.com/package/ical-browser

@github-actions
Copy link
Contributor

This issue has been automatically marked as stale because it has not had recent activity 😴
It will be closed in 14 days if no further activity occurs. To unstale this issue, add a comment with detailed explanation.
Thank you for your contributions ❤️

@github-actions github-actions bot added the stale label Jun 27, 2023
@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Jul 12, 2023
@sebbo2002 sebbo2002 reopened this Sep 16, 2023
@12joan
Copy link

12joan commented Sep 16, 2023

I also want to note that the proposed option with Webpack configuration does not work with Vite. Because of what, it is not possible to work in this collector. Instead, errors are thrown:

I'm using this patch (applied using patch-package) to make my app build correctly with Vite. It replaces the offending functions with ()=>{}. Hope it helps! ❤️

patches/ical-generator+5.0.1.patch

@sebbo2002 sebbo2002 removed the stale label Sep 16, 2023
@sebbo2002 sebbo2002 mentioned this issue Oct 19, 2023
github-actions bot pushed a commit that referenced this issue Oct 19, 2023
# [6.0.0-develop.1](v5.0.2-develop.2...v6.0.0-develop.1) (2023-10-19)

### Features

* Ensure Calendar is renderable all the time ([f1328a3](f1328a3)), closes [#344](#344)
* Remove `save()`, `saveSync()`, `serve()`, `toBlob()`, `toURL()` ([b6bea66](b6bea66)), closes [#478](#478)

### BREAKING CHANGES

* `Alarm.trigger` now defaults to 10min before event, `Alarm.type` now defaults to `display`, `Alarm.interval()` got removed, use `Alarm.repeat()` instead, `Alarm.repeat()` now gives/takes an object instead of a number, `Attendee.email` can’t be `null | undefined`, `Category.name` can’t be `null | undefined`, `Event.start` now defaults to now (`new Date()`). For details and examples checkout the migration guide at https://github.com/sebbo2002/ical-generator/wiki/Migration-Guide:-v5-%E2%86%92-v6
* The `save()`, `saveSync()`, `serve()`, `toBlob()` and `toURL()` methods of the ICalCalendar class have been removed. Please use the `toString()` method to generate the ical string and proceed from there.
@sebbo2002
Copy link
Owner

🎉 This issue has been resolved in version 6.0.0-develop.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

@sebbo2002
Copy link
Owner

I just pushed a preview of ical-generator v6 to develop, which should fix this issue. I would be happy if you could test the new version and give me some feedback. You can install the version with npm i ical-generator@next. All changes in this release can be found here.

@12joan
Copy link

12joan commented Oct 19, 2023

Upgrading to v6.0.0-develop.1 fixed the issue for me. Thanks! ❤️

github-actions bot pushed a commit that referenced this issue Oct 25, 2023
# [6.0.0](v5.0.1...v6.0.0) (2023-10-25)

### Bug Fixes

* add `browser` field to `package.json` ([7db4e32](7db4e32))

### Features

* Enable npm provenance ([87d173a](87d173a))
* Enable npm provenance ([ccba971](ccba971))
* Ensure Calendar is renderable all the time ([f1328a3](f1328a3)), closes [#344](#344)
* Remove `save()`, `saveSync()`, `serve()`, `toBlob()`, `toURL()` ([b6bea66](b6bea66)), closes [#478](#478)

### Reverts

* Revert "ci: Downgrade is-semantic-release till it's fixed" ([91c2ab5](91c2ab5))

### BREAKING CHANGES

* `Alarm.trigger` now defaults to 10min before event, `Alarm.type` now defaults to `display`, `Alarm.interval()` got removed, use `Alarm.repeat()` instead, `Alarm.repeat()` now gives/takes an object instead of a number, `Attendee.email` can’t be `null | undefined`, `Category.name` can’t be `null | undefined`, `Event.start` now defaults to now (`new Date()`). For details and examples checkout the migration guide at https://github.com/sebbo2002/ical-generator/wiki/Migration-Guide:-v5-%E2%86%92-v6
* The `save()`, `saveSync()`, `serve()`, `toBlob()` and `toURL()` methods of the ICalCalendar class have been removed. Please use the `toString()` method to generate the ical string and proceed from there.
@sebbo2002
Copy link
Owner

🎉 This issue has been resolved in version 6.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

No branches or pull requests

4 participants