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

Add untranspiled build and typescript support #177

Merged
merged 21 commits into from May 23, 2021
Merged

Conversation

sbarfurth
Copy link
Collaborator

@sbarfurth sbarfurth commented Nov 5, 2020

Description

This pull request adds a single build that completely untranspiled called v8n.esm.browser.js. This build can be used for modern browsers where backward compatibility is no needed. It also adds typescript typings that allow usage of this package in a typescript environment.

Fixes #165
Fixes #28
Fixes #180

Type of change

  • New feature (non-breaking change which adds functionality)

Checklist:

  • I have created my branch from a recent version of master
  • The code is clean and easy to understand
  • I have added changes to the Unreleased section of the CHANGELOG

@sbarfurth sbarfurth requested a review from imbrn November 5, 2020 15:00
@sbarfurth
Copy link
Collaborator Author

sbarfurth commented Nov 6, 2020

This pull request also changes the way prettier is applied to be in line with what is recommended. This means removing the plugin that runs prettier through eslint. Instead the config is used, meaning that formatting rules are disabled for eslint and left to prettier. The pre-commit hook was changed accordingly.

I've also added a .prettierrc that might be subject of discussion.

Copy link
Owner

@imbrn imbrn left a comment

Choose a reason for hiding this comment

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

Hey @brfrth, I liked these changes very much, mostly the prettier ones. I made just a comment that is very important about the generated files names.

rollup.config.js Outdated Show resolved Hide resolved
@sbarfurth
Copy link
Collaborator Author

sbarfurth commented Nov 8, 2020

@imbrn what's the current list of generated names for all builds?

@imbrn
Copy link
Owner

imbrn commented Nov 8, 2020

@brfrth

These are the file names generated by the build:

v8n.amd.js
v8n.browser.js (iife)
v8n.browser.min.js (iife minified)
v8n.cjs.js
v8n.esm.js
v8n.min.js (umd minified)
v8n.system.js
v8n.umd.js

the map files:

v8n.browser.min.js.map
v8n.min.js.map

and in the package.json file you can see which ones are used by our CDNs:

v8n/package.json

Lines 5 to 8 in 8b2e7c7

"main": "./dist/v8n.cjs.js",
"module": "./dist/v8n.esm.js",
"jsdelivr": "./dist/v8n.min.js",
"unpkg": "./dist/v8n.min.js",

@sbarfurth
Copy link
Collaborator Author

@imbrn fixed

@sbarfurth sbarfurth requested a review from imbrn November 10, 2020 17:03
package.json Outdated Show resolved Hide resolved
package.json Outdated Show resolved Hide resolved
rollup.config.js Outdated Show resolved Hide resolved
rollup.config.js Outdated Show resolved Hide resolved
rollup.config.js Outdated Show resolved Hide resolved
Copy link
Owner

@imbrn imbrn left a comment

Choose a reason for hiding this comment

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

@brfrth Please check the comments out.

@sbarfurth
Copy link
Collaborator Author

I fixed the stuff you reviewed. I also went ahead and updated all the npm packages used, since quite a few of them were outdated. In the process I was able to reduce the usage of babel by quite a bit. If we ever want to target a specific browser list we need it again, but for now it seems the goal is just an ES5 transpile. Many projects (e.g. Vue) use buble for this task as it much simpler than babel, so I went with it. We still need babel for jest, so it continues to be a dependency. The build config is massively nicer now, since it lacks the whole babel config.

I also updated the CircleCI config.

@sbarfurth sbarfurth requested a review from imbrn November 15, 2020 17:32
@sbarfurth
Copy link
Collaborator Author

I've pretty much copied Vue's approach, that should be the way to go.

@sbarfurth
Copy link
Collaborator Author

sbarfurth commented Nov 17, 2020

Added note about Typescript to the docs and changelog.

@sbarfurth sbarfurth added this to the v1.4.0 milestone Nov 17, 2020
@cyrilchapon
Copy link

IMHO; that's a 10-files project.
It could benefit from a full Typescript rewrite after this.

Interested into this ? I can give it a shot maybe

@sbarfurth
Copy link
Collaborator Author

@cyrilchapon I already had a full rewrite done. A full migration won't happen just now as per @imbrn comment on the matter. That said: You are free to do a TypeScript rewrite and let us know. If we ever look to migrate we can merge your branch.

@cyrilchapon
Copy link

I see. New transpilation stack will help anyways.

I might give that a shot if I can take time to.
I'll keep you up to date there

@sbarfurth sbarfurth linked an issue Nov 24, 2020 that may be closed by this pull request
types/v8n.d.ts Outdated Show resolved Hide resolved
types/test/test-v8n.ts Show resolved Hide resolved
@imbrn
Copy link
Owner

imbrn commented Apr 21, 2021

Hey @brfrth , I'm sorry for being all this time off.

Take a look at my last two comments.

@sbarfurth
Copy link
Collaborator Author

@imbrn Replied to both, resolved one. The other one you can decide on.

@sbarfurth
Copy link
Collaborator Author

@imbrn You want this squashed or merged as is?

@imbrn
Copy link
Owner

imbrn commented May 23, 2021

Yeah @barfurth , let's squash it before merging.

@sbarfurth sbarfurth merged commit a008df0 into master May 23, 2021
@sbarfurth sbarfurth deleted the feature/esm-build branch May 24, 2021 09:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Publish dist folder to git Optimise builds depending on consumer capability Support TypeScript
3 participants