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 IE polyfills and special casing #2947

Merged
merged 16 commits into from
Jun 28, 2021

Conversation

aduh95
Copy link
Member

@aduh95 aduh95 commented Jun 17, 2021

IE users would need to include abortcontroller-polyfill, math-log2, and url-polyfill from npm.

@aduh95 aduh95 added the 2.0 label Jun 17, 2021
Copy link
Member

@Murderlon Murderlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we document a list of all the polyfills required for IE somewhere? README or in the docs. I think it would save a lot of figuring out for people that do still need to support it.

@kvz
Copy link
Member

kvz commented Jun 17, 2021

Can we document a list of all the polyfills required for IE somewhere? README or in the docs. I think it would save a lot of figuring out for people that do still need to support it.

We should have a 1.0->2.0 migration guide (separate, or inlined with the announcement post), that lists all of these. I think it also makes sense to mirror the required polyfills to the general docs after that yes. It's never to soon to start tracking them in a central place indeed, wherever we end up copying it. Perhaps, the 2.0 branch can already contain a WIP post, and we keep a bullet list of these. Each PR that introduces new requirements bundles with it addition(s) to that list. By the time we're ready all that's left to do is cleanup the language and decide where else to publish.

MIGRATE_TO_V2.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@aduh95
Copy link
Member Author

aduh95 commented Jun 24, 2021

PR update:

  • documentation: ready for reviews
  • CDN bundles change:
    • uppy.js contains no polyfills, and contains modern-ish JS syntax.
    • uppy.ie.js contains all the polyfills, and is transpilled to ES5 – the name could be improved I think.
  • No E2E tests in IE anymore.
  • I've updated the E2E test platforms to use latest versions of the targeted browsers.
  • Some test suite now requires Node.js 15+ because of AbortController.

E2E tests are failing in Safari now, not sure if it's a flakiness of the CI or a real issue.

Copy link
Member

@Murderlon Murderlon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this all looks good but maybe @goto-bus-stop has an opinion on the bundling stuff.

@goto-bus-stop
Copy link
Contributor

uppy.ie.js contains all the polyfills, and is transpilled to ES5 – the name could be improved I think.

would just "uppy.legacy.js" be good or would you prefer something more specific?

@aduh95
Copy link
Member Author

aduh95 commented Jun 28, 2021

uppy.legacy.js is probably clearer indeed.
Any thoughts on what I did here? https://github.com/transloadit/uppy/pull/2947/files#diff-3efa66d67b2e7884e464b3a83b970becde04ff04229de4af90187f8b4f4db596

Copy link
Contributor

@goto-bus-stop goto-bus-stop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I think in the final blog post it would be good to make it extra clear that we used to include these polyfills always, so it's not like you suddenly need 20KB of polyfills that you didn't before; it's just that we always included them in the past and we now let you omit them if you don't need them

Copy link
Contributor

@goto-bus-stop goto-bus-stop left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just two inconsequential things ^^

bin/build-bundle.js Show resolved Hide resolved
bin/build-bundle.js Outdated Show resolved Hide resolved
aduh95 and others added 2 commits June 28, 2021 11:13
@aduh95 aduh95 merged commit c315329 into transloadit:2.0 Jun 28, 2021
@aduh95 aduh95 deleted the byebye-ie-see-you-never branch June 28, 2021 09:34
Murderlon added a commit that referenced this pull request Jun 28, 2021
* 2.0:
  Remove IE polyfills and special casing (#2947)
  Create `getObjectOfFilesPerState` in core for plugins (#2961)
Murderlon added a commit that referenced this pull request Jun 30, 2021
* 2.0:
  Remove IE polyfills and special casing (#2947)
  Create `getObjectOfFilesPerState` in core for plugins (#2961)
@aduh95 aduh95 mentioned this pull request Jun 30, 2021
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

Successfully merging this pull request may close these issues.

4 participants