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

dexie seems to be published as an ES6 module on NPM #195

Closed
oising opened this issue Mar 15, 2016 · 20 comments
Closed

dexie seems to be published as an ES6 module on NPM #195

oising opened this issue Mar 15, 2016 · 20 comments

Comments

@oising
Copy link

oising commented Mar 15, 2016

jspm_packages/npm/dexie-observable@0.1.4.js

export * from "npm:dexie-observable@0.1.4/dist/dexie-observable.es6.js";
export {default} from "npm:dexie-observable@0.1.4/dist/dexie-observable.es6.js";

Was this your intent? I'm relatively new to NPM but most (all?) seem to be ES5. Did you forget a transpilation stage or are you targeting node 5.x only?

@oising oising changed the title dexie-observable 0.1.4 seems to be published as an ES6 module on NPM dexie seems to be published as an ES6 module on NPM Mar 15, 2016
@oising
Copy link
Author

oising commented Mar 15, 2016

[16:47:11] Starting 'unbundle'...
[16:47:11] Finished 'unbundle' after 51 ms
[16:47:11] Starting 'bundle'...
[16:47:14] 'bundle' errored after 2.51 s
[16:47:14] Error: MultipleErrors: Error compiling global module "npm:dexie@1.3.3.js" at jspm_packages\npm\dexie@1.3.3.js
        jspm_packages/npm/dexie@1.3.3.js:1:1: Unexpected reserved word export
jspm_packages/npm/dexie@1.3.3.js:1:15: Semi-colon expected
jspm_packages/npm/dexie@1.3.3.js:2:1: Unexpected reserved word export
jspm_packages/npm/dexie@1.3.3.js:2:8: Semi-colon expected
jspm_packages/npm/dexie@1.3.3.js:2:9: Unexpected reserved word default
jspm_packages/npm/dexie@1.3.3.js:2:23: Semi-colon expected
...

This is the result of my trying to bundle it for a browser. I'm no expert, but should it be detected as a "global module?"

@oising
Copy link
Author

oising commented Mar 15, 2016

Yeah, there it is in your package.json:

"jspm": {
    "main": "dist/dexie.es6.js",
    "format": "esm"
  },

I would not expect an ES6 module here. This really should be ES5, no?

@dfahlander
Copy link
Collaborator

I'm also new to jspm, but I've tested successfully to jspm install dexie latest version (1.3.3).

jspm should handle ES6 modules. That's why I point out the es6 module and format "esm" which is JSPM's alias of ES6.

There was a jspm bug in 1.3.2. Check your package.json's jspm-dependencis so that it is not tied to an old version.

//package.json:
  ...
  "jspm": {
    "dependencies": {
      "dexie": "npm:dexie@^1.3.3"
    },

@oising
Copy link
Author

oising commented Mar 16, 2016

Yes, JSPM can handle es6 modules if you configure a transpiler, but I don't use one because hosting a transpiler at runtime in a browser is expensive, and I don't need one at design time because I'm using TypeScript. This isn't really a question about JSPM though. It's more about conventions for packages placed on npmjs.org. Out of the hundreds and hundreds of packages I'm using directly (top level) and indirectly (dependencies), yours is the only one published as ES6 (you are obviously free to write it in ES6.) Do I really have to turn activate the transpiler just for yours? :)

I guess the real question is this: Isn't the convention to publish ES5 on npmjs? Your development cycle can include ES6/ES5 transpilation, but why enforce your choice on everyone else?

Ultimately your package is targeted at browsers. Browsers don't run ES6 yet, and we won't have wide coverage for a long time. My workflow is to use TypeScript in VS, and I love your TS support but I don't have a transpilation pipeline nor have I ever needed one until now.

Thanks again for your amazing work -- I absolutely love (nearly) everything about what you've done. :)

@dfahlander
Copy link
Collaborator

Aha, thanks much for the explanation! Then I suppose I should delete the jspm element in package config, and jspm should pick the main element - the es5 module and not try to confuse jspm.

Thanks a lot for liking my work :)

Cheers!

@oising
Copy link
Author

oising commented Mar 16, 2016

Just to reassure myself, I did some research and the general consensus is that people still publish as ES5... it'll be a while yet. Here's a good article how to help set up your workflow to write in ES6 and publish in ES5

https://booker.codes/how-to-build-and-publish-es6-npm-modules-today-with-babel/

@oising
Copy link
Author

oising commented Mar 16, 2016

I'm about to dive head-first into Dexie.Syncable ... wish me luck! First step is to port your node server to c# -- I'll submit a pull request to you when I get it working if you'd like to integrate it into your samples.

@dfahlander
Copy link
Collaborator

Please do, it would be much apprecieated. If it would target a real database that would be even greater. I myself am depending on Dexie.Syncable for my app and I have a backend with lots of legacy code and dependencies to my app - that's why I couldnt publish it. However, it actually does its job. I still use ajax sync instead of websocket. My plan is to go over to socket.io or SignalR. If you're on C#, I would recommend SignalR on the server. We used it with success on another project I was on last year. Very easy API.

I'm not so proud of the code structure in Dexie.Observable and Dexie.Syncable so I hope to get time to rewrite them. Dexie.Observable is currently dependent on "onstorage" event for communicating changes between browser windows. The issue with that is that localStorage is not present in Web Workers, so as long as I don't find a fallback for that, the addons cannot be used from a Web Worker.

The principle of Dexie.Observable and Syncable are quite easy. Most of the work is done in Dexie.Observable:

  • A "changes" table that works as an oplog for the DB.
  • A "nodes" table where each node represents either a locally connected Dexie instance, or a remote sync server. Each node has a revision number. The revision number is the same as the primary key on the changes table in the point of time that the node was updated. All changes since then is what that node will get on next sync.
  • An "uncommittedChanges" table that is a temp storage while downloading changes from the server.

However, the code would need to be restructured and hopefully possible to simplify a lot.

@oising
Copy link
Author

oising commented Mar 16, 2016

Hah! I originally was going to use SignalR for the change broadcasting but I was expecting to have to write the framework myself. It was then that I found Dexie and it didn't really enter my head to use SignalR as a transport but if you're saying it is viable, then I may go forward with that. Even writing it out now is helping solidify the thought. It's nice to have SignalR abstract out the transport and then we can just focus on data.

Whatever I learn, I'll do my best to share back this direction and perhaps we may have a chance in the future to work on it together.

@dfahlander
Copy link
Collaborator

I'm having an issue with jspm (and that was why I started adding jspm element to package.json). If I do jspm install npm:dexie@1.3.1 which was a version before I started adding jspm to package.json, I get:

MultipleErrors: C:/Users/David/AppData/Local/.jspm/packages/npm/dexie@1.3.1/dist/dexie.es6.js:3704:1: Unexpected reserved word export
C:/Users/David/AppData/Local/.jspm/packages/npm/dexie@1.3.1/dist/dexie.es6.js:3704:8: Semi-colon expected
C:/Users/David/AppData/Local/.jspm/packages/npm/dexie@1.3.1/dist/dexie.es6.js:3704:8: Unexpected reserved word default
C:/Users/David/AppData/Local/.jspm/packages/npm/dexie@1.3.1/dist/dexie.es6.js:3704:16: Semi-colon expected

It seems it tries to interpret dexie.es6.js as an ES5 module. These errors are gone in 1.3.3. Tried to laborate with this by using the -o argument:

jspm install npm:dexie -o "{main:'dist/dexie.js', format: 'cjs' }"

...but same result.

Ok, now I got it to install without errors:

jspm install npm:dexie -o "{main:'dist/dexie.js', format: 'cjs', ignore: ['dist/dexie.es6.js'] }"

So I will actually need the jspm element but with just an ignore in it, I suppose.

@oising
Copy link
Author

oising commented Mar 16, 2016

hmm... but really, should there be es6 code in the package at all? It's my understanding that the es6 stays in github and your npm publish process only publishs es5...

I mean, right now all I have to do with either npm or jspm is just jspm install npm:dexie without any mapping or extra work, and it should work without having to invoke babel/traceur to transpile. I have the transpiler explicitly set to false in my config.js:

System.config({
  baseURL: "/",
  defaultJSExtensions: true,
  transpiler: false,
  paths: {
    "github:*": "jspm_packages/github/*",
    "npm:*": "jspm_packages/npm/*"
  },

  map: {
    ...

and every other one of the hundred-odd packages I'm using all work in my browsers (chrome, ie, safari etc.) without any transpilation.

edit: I wish I could help - I'm just trying to explain the end-user experience expected :)

@dfahlander
Copy link
Collaborator

Yes, your help is appreciated. And you may be right. The reason for including es6 is to support a better experience for other people writing es6 code and including dexie. That was one of the main reasons for migrating to ES6 in the first place (#140). They can use rollup to bundle their code and it will understand jsnext:main attribute. But it may be overkill. For jspm at least.

@oising
Copy link
Author

oising commented Mar 17, 2016

In #140 @wgebczyk suggests "write Dexie in es6 and transpile to es5" hehe ;)

But I see what you're trying to do. Is it possible to publish es5 + es6 in the same package? That's what it seems to allude to, but something isn't right...

@dfahlander
Copy link
Collaborator

Well it's what rollup suggests, using the jsnext:main property. And rollup is increasing in popularity.

@dfahlander
Copy link
Collaborator

From the rollup README:

"Can I distribute my package as an ES6 module?

If your package.json has a jsnext:main field, ES6-aware tools like Rollup can import the ES6 version of the package instead of the legacy CommonJS or UMD version. You'll be writing your code in a more future-proof way, and helping to bring an end to the dark days of JavaScript package management"

https://www.npmjs.com/package/rollup#can-i-distribute-my-package-as-an-es6-module

https://github.com/rollup/rollup/wiki/jsnext:main

@oising
Copy link
Author

oising commented Mar 17, 2016

Cool -- that's good to know. So did you publish a fixed version on NPM yet? :)

btw, I have been looking into making Dexie Syncable work over SignalR and https://github.com/AndersMalmgren/SignalR.EventAggregatorProxy -- what do you think of the idea?

@dfahlander
Copy link
Collaborator

Just published 1.3.4. Actually removed the es6 output from dist and included src instead. Sounds interesting with the EventAggregatorProxy - looking at it briefly seems like something smart when having multiple types of signalr events going on.

Would you be able to verify whether the new package works for you now? I tested to jspm install npm:dexie and it succeeds, but there's still an error file under the package .jspm.errors where it complains about all src and test files because they're in ES6 format.

If the jspm install works for you and you can require dexie, I could close this issue. Otherwise I'll have to update the jspm.ignore attribute. Have "src" there now, but it didn't bite. By labbing with jspm install with the -o flag, it turns out that the only way to get rid of this error file would be to explicitely list all individual files to IGNORE. I would not want to have to maintain that list just to make jspm error file go away, so if it works for you to use dexie the way you want, I will close this issue despite the error file that jspm generates.

@dfahlander
Copy link
Collaborator

Filed an issue for this at jspm jspm/jspm-cli#1624

@dfahlander
Copy link
Collaborator

Ok, ive tried to install dexie via jspm with transpilation off using a simple hello world approach. I could install and run dexie from my app. Closing this issue.

@oising
Copy link
Author

oising commented Mar 22, 2016

Thank you for your attention and the prompt turnaround! Much appreciated!

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

No branches or pull requests

2 participants