search

abramenal / cypress-file-upload

File upload testing made easy
https://npm.im/cypress-file-upload
MIT License
496 stars 89 forks source link

[Feature] Cypress adding native attach file support in 9.3.0 #338

Closed BlueWinds closed 1 year ago

BlueWinds commented 2 years ago

cypress-file-upload has a variety of issues with file encoding, and an ongoing support log about working with binary files. This is understandable - cypress has historically been very text-centered, and not had much interest in supporting binary files.

In 8.8.0, cypress will be adding the option to pass null as an explicit encoding, which will return a Buffer directly, no base64 encoding intermediary. This ought to simplify working with binary files / fixtures. See https://github.com/cypress-io/cypress/pull/18534 for the implementation and some discussion, or https://github.com/cypress-io/cypress-documentation/pull/4172 for the related documentation changes.

This isn't a feature request so much as it is a conversation starter - are there any further changes to cypress' internals that would make file upload support easier? This plugin is the best existing solution, and as we start looking into adding native Cypress support for file uploads, your experience and insight would be welcome.

Versions

Cypress 8.8.0, not released yet.

abramenal commented 2 years ago

Hi @BlueWinds 👋 Removing intermediary base64 encoding indeed sounds like a nice improvement.

From my perspective, the biggest points are:

Note: different file drop UI libraries rely on its own implementations, and not all of them follow the standards/specs. This was the main reason for me to have recipes where I can actually test things on the actual UI libraries. Another quirk here is different browser-specific things like https://github.com/abramenal/cypress-file-upload/issues/320

So I think an abstraction around files that a user attaches to certain element would be useful here.

While most of the users don't really care about correct encodings, often incorrect encoding affects other parts of user applications (eg backend responds incorrectly if a file is corrupted). So it would be extremely beneficial to have support for most used file types to let users focus on user behavior and not File entity specs.

Often ppl do silly things, and this is fine. When attaching a file to an element that is not supposed for that, it should be clear how to find the correct element. Similarly, when there is a problem with file encoding, user should have ability to further investigate/find proper solution for its specific use case.

These are some high level thoughts on this. Would love to be useful here, so let me know if I can help somehow with that. I believe file upload is an absolutely necessary feature, so I'm glad that Cypress team sees it as a potential built-in.

BlueWinds commented 2 years ago

I've started work on the implementation - https://github.com/cypress-io/cypress/pull/18825 is the current PR, would welcome a review if you'd like. This is just the simplest implementation so far, without any thought with how files are loaded - there are follow-up tickets in https://github.com/cypress-io/cypress/issues/18753 and https://github.com/cypress-io/cypress/issues/18754.

The current thought was that we'd credit you and this project, a'la https://github.com/cypress-io/cypress/tree/develop/npm/cypress-schematic#community-recognition, if that's something you're interested in. We probably won't be pulling and code in from cypress-file-upload directly, but this was definitely my starting point for researching the feature, and we'll be using something pretty similar to your API to make it easy for users to migrate over.

BlueWinds commented 2 years ago

I also wrote a fairly in-depth technical brief, which goes over a lot of our internal discussion around the feature. It goes over the full plan, even though the current PR doesn't implement loading files from disk yet.

https://github.com/cypress-io/cypress/blob/issue-18752-attachFile/packages/driver/src/cy/commands/actions/attach-file-tech-brief/README.md

abramenal commented 2 years ago

Sounds good! I'll try to review the tech proposal this week. As for credits – fine with me. Once this is usable as an internal cypress module, I'll deprecate this package & create a migration guide, if that will be necessary.

BlueWinds commented 2 years ago

Ok, some more updates. We have finalized (or nearly finalized) PRs for both the code itself and four our updated documentation. Aiming to get this in the 10.0 release, scheduled tentatively for mid-January.

Code: https://github.com/cypress-io/cypress/pull/19332 Documentation, including a migration guide for users of cypress-upload-file: https://github.com/cypress-io/cypress-documentation/pull/4241

BlueWinds commented 2 years ago

Update: I've renamed our version from attachFile to selectFile so that there won't be any conflict - users of this plugin won't be forced to switch over or update anything, it'll just keep working as expected. Not using the same name should make things smoother for everyone involved.

With that done, we've also moved this to the 9.3 release, which should go out around the 18th. New PR for develop branch: https://github.com/cypress-io/cypress/pull/19524

abramenal commented 2 years ago

Cool stuff @BlueWinds, it is finally released! I am really glad that it's finally an out-of-box feature. Long-awaited, I must say 🥇

This also means it's been almost 3-year-long journey for the package (first commit made on Feb '19).

I will be preparing some deprecation notes as well as migration guide. But I assume this has to be published once the built-in feature is being live for a bit. Let's hear if the users love it.

Do you have any idea when this package has to be deprecated? I see it is actually being mentioned as deprecated here, but I assumed there should be some room for initial bugfix/maintenance. Or is it already considered as fully stable? I am especially curious about possible cross-browser issues, also encodings support.

BlueWinds commented 2 years ago

So first off, there are a couple of missing features with the 9.3.1 release.

Both these will be addressed in 9.4.0, which should be coming out early next week. Some users are already switching over, but these two are blockers for tons of folks.

With that said, cypress-file-upload will continue to work - there's no plans to break it, and users should have as much time as they like to move over. The docs call it deprecated in the sense of "we think you'll like using selectFile better", not in the sense of "it will be broken and no longer supported soon." I just wrote that in the docs so that I wouldn't have to go back and update them later - that sort of thing always gets forgotten about.

BlueWinds commented 2 years ago

We also wrote up a migration guide - any feedback is appreciated, of course.

https://docs.cypress.io/guides/references/migration-guide#Migrating-from-cypress-file-upload-to-selectFile

The bit about mimeType being unsupported will be updated when it actually is supported. :) (docs PR is here: https://github.com/cypress-io/cypress-documentation/pull/4321/files#diff-50ef08355b192e64726bbf1dd0e050ca1f1612d1472de477168d37baa529bf5c)