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 all IIFEs #1172

Merged
merged 1 commit into from
Jan 11, 2020
Merged

Remove all IIFEs #1172

merged 1 commit into from
Jan 11, 2020

Conversation

nicksellen
Copy link
Contributor

@nicksellen nicksellen commented Jan 9, 2020

Now we are in npm/webpack world we don't need these to give us scope

Proposed Changes

I wanted to get rid of the all the angular-era IIFE's (you know, (function(){ /* code goes here */ })() that wraps everything) as they are no longer needed we are in webpack (each file is it's own scope).

I wrote a jscodeshift script to do it (see https://gist.github.com/nicksellen/cbe11ad0ca9073ce762c02c897e67d1c), I was thinking we can just keep it as a standalone script (not in repo), make one BIG change PR to change everything, but then people can run the script in their branches to minimise big change.

Whaddya think?

Testing Instructions

  • site should work as before!

To run the script:

npm install -g jscodeshift
curl https://gist.githubusercontent.com/nicksellen/cbe11ad0ca9073ce762c02c897e67d1c/raw/cf137f497d2d891d42c02cc6260e13f46f8f6d9f/transform.js > /tmp/transform.js
jscodeshift --no-babel -t /tmp/transform.js modules

Fixes #

Now we are in npm/webpack world we don't need these to give us scope
Copy link
Contributor

@simison simison left a comment

Choose a reason for hiding this comment

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

Let's get this in — in our PRs we're mostly removing Angular code so rebasing should be kinda straightforward.

I smoke tested that the site works.

Good to hear from @mrkvon first tho since he has lots of lines of code in the rebase game. :-)

@nicksellen nicksellen marked this pull request as ready for review January 9, 2020 22:24
@nicksellen nicksellen requested a review from mrkvon January 9, 2020 22:24
Copy link
Contributor

@mrkvon mrkvon left a comment

Choose a reason for hiding this comment

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

When one hides whitespace changes, this PR is pretty straightforward. 👌
I'm fine with rebasing whatever. Let's merge this.

@simison simison merged commit ea51189 into master Jan 11, 2020
@simison simison deleted the remove-iifes branch January 11, 2020 16:01
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.

3 participants