iLoveElysia/pjax
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 9 Wiki Activity

ES6 syntax in compiled build breaks compatibility with older browsers #106

New Issue
Closed
opened 2018-01-18 15:36:49 -05:00 by robinnorth · 2 comments
robinnorth commented 2018-01-18 15:36:49 -05:00 (Migrated from github.com)
Copy Link

It looks like some of the more recent changes to this library have introduced the use of the const keyword e.g. the in new switchElementsAlt method: b98e3ef914/lib/switches.js (L23)

While this is not an issue of itself, the npm prepublish script doesn't include a transpiling step, meaning that these const keywords are still present in the compiled pjax.js build artifact.

To fix this and continue using ES6 syntax, the prepublish step could use Babel in conjunction with Browserify. Alternatively, the 3 instances of const in use could be replaced with var keywords.

It looks like some of the more recent changes to this library have introduced the use of the `const` keyword e.g. the in new `switchElementsAlt` method: https://github.com/MoOx/pjax/blob/b98e3ef9144553e3e8c70c82b04b5220f2dab589/lib/switches.js#L23 While this is not an issue of itself, the npm prepublish script doesn't include a transpiling step, meaning that these `const` keywords are still present in the compiled `pjax.js` build artifact. To fix this and continue using ES6 syntax, the prepublish step could use Babel in conjunction with Browserify. Alternatively, the 3 instances of `const` in use could be replaced with `var` keywords.
BehindTheMath commented 2018-01-18 15:41:36 -05:00 (Migrated from github.com)
Copy Link

You're right, that's a bug. While it would be great to use ES6, a big part of Pjax is being compatible with older browsers, and I don't think it's makes sense to add extra tooling at this point.

You can submit a PR, or I'll fix it when I have a chance.

You're right, that's a bug. While it would be great to use ES6, a big part of Pjax is being compatible with older browsers, and I don't think it's makes sense to add extra tooling at this point. You can submit a PR, or I'll fix it when I have a chance.
robinnorth commented 2018-01-18 15:44:05 -05:00 (Migrated from github.com)
Copy Link

@BehindTheMath I agree that it seems like a lot of effort to add extra tooling to transpile 3 keywords, so I will submit a PR for you. I'm currently using the library in a project I'm developing and was very pleased to see your efforts in working towards pushing out a new version on npm -- I'm very happy to do anything I can to help move that along 😄

@BehindTheMath I agree that it seems like a lot of effort to add extra tooling to transpile 3 keywords, so I will submit a PR for you. I'm currently using the library in a project I'm developing and was very pleased to see your efforts in working towards pushing out a new version on npm -- I'm very happy to do anything I can to help move that along 😄
👍 1
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
bug
dependencies
Pull requests that update a dependency file
 duplicate
enhancement
invalid
question
wontfix
No Label bug
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
iLoveElysia
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: iLoveElysia/pjax#106
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?