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

Three improvements for the price of one #291

Merged
merged 5 commits into from
Mar 7, 2019
Merged

Three improvements for the price of one #291

merged 5 commits into from
Mar 7, 2019

Conversation

favna
Copy link
Contributor

@favna favna commented Mar 3, 2019

This PR aims to improve three things in this lib

  1. I have cleaned up the build as well as the build scripts in b8b562c. What this means is

    • Cleaned up unnecessary packages
    • Updated existing packages
    • Cleaned up Webpack configuration and standardized it in accordance with the docs
    • Cleaned up NPM scripts
    • Remove package-lock.jon in favor of yarn.lock, either way only 1 lockfile should ever be present. I've opted for yarn.lock as package-lock has a tendency to change between operating systems (I experience this at work due to some devs working on Linux and others on MacOS) which creates unnecessary commit bloat
    • Specify "files" in package.json, remove npmignore. It is more consistent and doesn't rely on changing a file all the time. Read this medium article for more info. TLDR: Specify what you want, not what you don't want.
    • Moved typings to src
    • Use terser-webpack-plugin to compress and mangle production builds
    • Properly clean dist between builds
    • Properly split dev and prod builds, no using of .min.js when it is not being referenced by the main field in package.json
    • Use webpack to copy typings into dist, this is a side-effect of properly using "files" in package.json
    • Cleaned up gitignore
      • Specified more IDE settings
      • Filter all log files, not just npm-debug.log
      • And some other fluff
  2. I have rewritten all tests into the Jest framework in c2b4075. This is primarily due to the fact that Vows was not capable of testing TypeScript which I needed for point 3, however while porting the tests I also noticed many issues with the current ones. Ranging from describing A but doing B, duplicating tests or tests that should not have passed at all, let me just say it was a mess. If anything by merging this PR the lib will be way better tested.

  3. I have improved the TypeScript typings with a way that finally allows us TypeScript users to take full control over our typings in 86b420b. This resolves FuseOptions<T> prevents nested key search #261, a longstanding issue concerning TypeScript usage. In order to hopefully ensure that TypeScript usage will stay working like it is now I have also written an extra test suite for it. It should be noted that very shortly after opening this PR I will also create a PR for the gh-pages branch which adds TypeScript usage to the examples.

This PR bumps the version patch number from 3.4.2 to 3.4.3. I have also updated the CHANGELOG.md to reflect all the changes made by this PR.

I think that's about everything covered. If you have any questions or concerns just let me know and I'll address them at my earliest possibility.

favna added 4 commits March 3, 2019 16:24
Partial: Requires next commit for a proper version!
This should resolve #261 as long as the system of the test is being used. That being said, this system allows for a fully typed configuration with IDE support

TypeScript usage is further described in the documentation, so please accept that PR if this one is accepted.
@krisk
Copy link
Owner

krisk commented Mar 7, 2019

Hi @favna, thanks for taking this on. This is wonderful! Currently looking over the changes now.

@krisk krisk merged commit 48e55f7 into krisk:master Mar 7, 2019
@StephenAWalsh
Copy link

StephenAWalsh commented Mar 7, 2019

Just found out 3.4.3 fails in IE11 whereas 3.4.2 does not. I think this might be due to your webpack/transpilation changes.

This is because fat arrows end up in dist code.

3.4.2
image

vs

3.4.3
image

@favna
Copy link
Contributor Author

favna commented Mar 7, 2019

I admittedly did not expect (many) users to use the lib for an IE11 production environment. You are totally correct that it is due to the arrow function. Sadly that cannot be polyfilled either. I'll look into this since it'll require modifying Terser options and create a new PR to cover it once I figure it out. Cannot make any ETA promise however. Probably somewhere this weekend.

@StephenAWalsh
Copy link

We have a legacy app that still has 15% of users running IE11 (it's a total pain!!!). I just forcefully changed my dependency back to 3.4.2. Appreciate your quick attention!!

@favna
Copy link
Contributor Author

favna commented Mar 7, 2019

Don't worry, I know the struggle. My own job is a legacy project too and I use one of my own libs in it as well where I actually made the exact same mistake but there it was a config in my TypeScript configuration.

(we got like 80% working in IE despite Firefox being available ╯°□°)╯︵ ┻━┻)

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.

FuseOptions<T> prevents nested key search
3 participants