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

V2-rc1 #463

Merged
merged 6 commits into from
Sep 26, 2018
Merged

V2-rc1 #463

merged 6 commits into from
Sep 26, 2018

Conversation

jimmywarting
Copy link
Collaborator

@jimmywarting jimmywarting commented Sep 21, 2018

I think the issues have been overwhelmed by reports and i would want to try to close most of them once and for good to give it a more clean slate. (Many issues have been stale for a long time and i can't determinate if it's relevant anymore. Would like to close all (most) issue for being too old. Anything still an issue can be reopen or made as a new issue)

I have try to go throught all of the issues. Trying to clean up things and rewrite the hole thing from the ground up to try and fix most pr & issues for 3 days now

here are some changes:

  • Removed the hole event thing since nobody is using it anyway
    • ppl only do saveAs(...) so there is no reason to return anything
    • Detecting any of those event have never or will never be possible to detect (onwrite, onend, etc) since the spec was abandoned.
  • Removed the demos since it's rather old (lets keep it to readme's wiki's and jsfiddles)
    • Means less maintainability
  • Removed date/version from top of the file since it easy to forget to change
  • Dosen't crash in web workers
  • Support saving urls (Use URL as data #260 with workarounds for cross origin)
  • Uses babel universal module pattern (UMD)
  • provides source map now as well.
  • use a[download] before msSaveAs (Not work with lumia! #193, Workaround for Windows phone 10 Edge bug (#193) #294)
  • removed dist from .gitignore (npm uses it if it don't find a .npmignore)
  • use conditional function instad of trying every possible solution in one flow (future detection)
  • noAutoBom switched to autoBom (remove BOM? #432 makes autoBom optional)
  • no longer use dispatchEvent since there are new and depricated event constructors that works differently (Unable to dispatch click on Chrome/Safari #382)
  • opens up a new popup (tab) directly for the fallback method since the FileReader is async
  • removed the explicitly MSIE [1-9] check since blob.js can polyfill them but i doubt anyone are still using such old browser anymore that made the check necessary.

closes #382
closes #460
closes #459
closes #294
closes #193
closes #260
closes #142
closes #449
closes #432
closes #416
closes #415
closes #409
closes #346
closes #260
and possible more

hopes to solve (#453, #451, #445, #441, #438, #436, #433, #434, #431, #408, #427, #423, #414, #422, #414, #248 and possible more)

@jimmywarting jimmywarting self-assigned this Sep 21, 2018
@jimmywarting jimmywarting added this to the v2 milestone Sep 21, 2018
@Ruffio
Copy link

Ruffio commented Sep 21, 2018

Cool. Can't wait to test it when release :-) Makes good senge to start all over.

@jimmywarting
Copy link
Collaborator Author

jimmywarting commented Sep 21, 2018

@Ruffio glad to here, will see if @eligrey is onboard with this as well, if so I might push it to https://www.npmjs.com/package/file-saver as -rc.1 so ppl can test it first.

Would be grate if you could test it out as much as possible on different browsers. Both url and blobs. and if the umd pattern works okey with import/require etc, I don't have IE/Edge on Mac so...

Any code review is appreciated.

didn't want to go ahead with igords promise based approach as IE don't have it so. es6 was also lite over the top since it's such a small self contained module. es6 would just increase the dependency of other modules and make it more complex

@eligrey
Copy link
Owner

eligrey commented Sep 21, 2018

The PR description sounds good to me! I have yet to review the code and will try to do that tomorrow.

@jimmywarting
Copy link
Collaborator Author

If you don't like the code formatting or something is done incorrectly, by all mean fork the PR, or write code review and i will change it.

I mostly stick to what StandardJS dose but i don't enforce a Linting as rules are often broken or things don't look the way you want it to be my and bitinn two cents: node-fetch/node-fetch#303 (comment)

@jimmywarting
Copy link
Collaborator Author

jimmywarting commented Sep 21, 2018

maybe thinking we can maybe squeeze in one small .d.ts file also for type definition to make other happy (it's just one function)
what i don't want to do is to write the code with typescript

Never liked typescript (only adds overhead and don't work in browser or node as it is. always need a extra compile step)

@eligrey
Copy link
Owner

eligrey commented Sep 26, 2018

lgtm

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