Feat/add diagnostics_channel shim #21
Conversation
JayaKrishnaNamburu
force-pushed
the
feat/add-diagnostics_channel-shim
branch
from
4a3bd14 to
b023447
Compare
src-browser/diagnostics_channel.js
Outdated
| @@ -0,0 +1,4 @@ | |||
| import dc, { channel, hasSubscribers, Channel } from "diagnostics_channel"; | |||
There was a problem hiding this comment.
@guybedford the build seems to fail. Looks like it is trying to resolve diagnostics_channel from node_modules/@jspm/core instead of node_modules/diagnostics_channel to add it in scope.
Here is the error
[!] (plugin @jspm/plugin-rollup) Error: ENOENT: no such file or directory, lstat '/Users/jkrishna/Documents/jspm/jspm-core/node_modules/@jspm/core/nodelibs/browser/diagnostics_channel.js'
Error: ENOENT: no such file or directory, lstat '/Users/jkrishna/Documents/jspm/jspm-core/node_modules/@jspm/core/nodelibs/browser/diagnostics_channel.js'
There was a problem hiding this comment.
Yes this is because in Node.js core modules take precedence over installed ones. Instead the trick here is to reference ../node_modules/diagnostics_channel/....js in the import.
There was a problem hiding this comment.
But it's trying to resolve from node_modules/@jspm/core/nodelibs/browser/diagnostics_channel.js right. Under @jspm/core of previous version. Which i have 2.0.0-beta.8 inside my node_modules. I think it is @jspm/plugin-rollup that is pulling the lastest version of @jspm/core available from npm.
@jspm/core@2.0.0-beta.8 /Users/jkrishna/Documents/jspm/jspm-core
└─┬ @jspm/plugin-rollup@1.1.0 (git+ssh://git@github.com/jspm/rollup-plugin-jspm.git#6da8627c351e216109fe8817232c99e23dae8324)
├── @jspm/core@2.0.0-beta.8
└─┬ @jspm/generator@1.0.0-beta.11
└── @jspm/core@2.0.0-beta.8 dedupedThere was a problem hiding this comment.
I got it now, the rollup-jspm-plugin is trying to pull from @jspm/core which don't don't have in the older version. And yes, resolving from ./node_modules solves the issue 👍
| @@ -109,6 +110,11 @@ | |||
| "deno": "./nodelibs/browser/perf_hooks.js", | |||
| "node": "./nodelibs/node/perf_hooks.js", | |||
| "default": "./nodelibs/browser/perf_hooks.js" | |||
| }, | |||
| "./nodelibs/diagnostics_channel": { | |||
| "deno": "./nodelibs/browser/diagnostics_channel.js", | |||
There was a problem hiding this comment.
Forcing to use browser for deno too. Because, i highly doubt deno will be accepting PR's that uses external packages as dependencies.
JayaKrishnaNamburu
changed the title
src-browser/diagnostics_channel.js
Outdated
| export default { | ||
| channel, | ||
| Channel, | ||
| hasSubscribers | ||
| }; |
There was a problem hiding this comment.
Should this just be export default dc?
There was a problem hiding this comment.
That's a weird bug, is it a Rollup bug? Did you try import dc from '../node_modules/diagnostics_channel/index.js? CommonJS modules (which this is) should always be imported that way.
There was a problem hiding this comment.
It happens only when i import like import dc from 'diagnostics_channel. But since i am importing using like
import * as dc ... now it works though
There was a problem hiding this comment.
Hmm, that's wrong though... we should fix the builder then.
There was a problem hiding this comment.
It could be an issue for https://github.com/jspm/rollup-plugin-jspm
There was a problem hiding this comment.
What I mean is using import dc should work and that it isn't working is a bug!
There was a problem hiding this comment.
Or rather the import dc form.
There was a problem hiding this comment.
So no bug there, the build was failing because, i missed index.js extension. But instead used just './node_modules/diagnostics_channel' for the import.
There was a problem hiding this comment.
Great, it sounds like if anything the jspm-rollup issue might be to ensure a clearer error message in this case then?
There was a problem hiding this comment.
Definitely, i will check if i can do something and throw some useful error message there then 👍
JayaKrishnaNamburu
force-pushed
the
feat/add-diagnostics_channel-shim
branch
from
8f4cbd9 to
23d1ef9
Compare
|
Great, thanks! |
#5