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

Cannot find module nlp #443

Closed
tgrosinger opened this issue Dec 14, 2020 · 7 comments · Fixed by #444
Closed

Cannot find module nlp #443

tgrosinger opened this issue Dec 14, 2020 · 7 comments · Fixed by #444

Comments

@tgrosinger
Copy link

tgrosinger commented Dec 14, 2020

rrule version: 2.6.6
OS: OSX
$ date: Sun Dec 13 19:40:34 PST 2020

Output

Not sure why this imports are unhappy here. This is from the console in an Electron app:

internal/modules/cjs/loader.js:798 Uncaught (in promise) Error: Cannot find module './nlp'
Require stack:
- electron/js2c/renderer_init
    at Module._resolveFilename (internal/modules/cjs/loader.js:798)
    at Function../lib/common/reset-search-paths.ts.Module._resolveFilename (reset-search-paths.ts:40)
    at Module._load (internal/modules/cjs/loader.js:691)
    at Module._load (electron/js2c/asar.js:717)
    at Function.Module._load (electron/js2c/asar.js:717)
    at Module.require (internal/modules/cjs/loader.js:853)
    at require (internal/modules/cjs/helpers.js:74)
    at w (app.js:1)
    at s (app.js:1)
    at getnlp (eval at <anonymous> (app.js:1), <anonymous>:14790:23)

Code

The error occurs when I call this (TypeScript):

import RRule from 'rrule';
// also tried
// import RRule from 'rrule/dist/esm/src/rrule';

...

const schedule = RRule.fromText("Every Sunday");

Build

Typescript with rollup.

rollup.config.js

import typescript from '@rollup/plugin-typescript';
import {nodeResolve} from '@rollup/plugin-node-resolve';
import commonjs from '@rollup/plugin-commonjs';

export default {
  input: 'src/main.ts',
  output: {
    dir: '.',
    sourcemap: 'inline',
    format: 'cjs',
    exports: 'default'
  },
  external: ['obsidian'],
  plugins: [
    typescript(),
    nodeResolve({browser: true}),
    commonjs(),
  ],
};
@olemartinorg
Copy link

Have a look at my comment in #430, this seems to happen because of a runtime import that @rollup/plugin-commonjs fails to identify.

@tgrosinger
Copy link
Author

@olemartinorg, thanks for the response! Should I try the rollup-plugin-commonjs-alternate, or would you recommend just waiting to see if you're able to change the circular dependency?

@olemartinorg
Copy link

@tgrosinger It doesn't work any better with rollup-plugin-commonjs-alternate, as that one creates a hard error on JS parsing when importing RRule. Rather test my fix by applying the pull request in #444 and let me know if that works for you.

@tgrosinger
Copy link
Author

I apologize for the naive question here, I'm mostly a backend developer and am new to the JS/TS world.

Is there a good way to apply that PR when I have installed the module through npm/yarn? I only have the transpiled files and types, so I don't even have nlp/index.ts and rrule.ts.

Would it work to do something like this?

yarn remove rrule
yarn install https://github.com/jakubroztocil/rrule/pull/444/commits/0f19f34cd7ff186df856477c65eb8717073a096c

@olemartinorg
Copy link

Sure! I recently delved into this JS/TS world myself.

I'm using npm instead of yarn in my project, but I see that RRule is using yarn. The way I would do it (roughly, as I haven't gotten there yet myself):

git clone https://github.com/olemartinorg/rrule.git # Or clone jakubroztocil/rrule and apply the pull request
cd rrule
yarn install
yarn run build

and then, in your project:

npm uninstall --save rrule
npm install --save file://../path/to/rrule

@tgrosinger
Copy link
Author

Awesome, I will give that a try this evening and report back. Thank you!

@tgrosinger
Copy link
Author

Your fix worked! As you mentioned in the MR, it still complains about some circular dependencies, but it does work.

src/main.ts → ....
(!) Circular dependencies
node_modules/rrule/dist/esm/src/index.js -> node_modules/rrule/dist/esm/src/rrule.js -> node_modules/rrule/dist/esm/src/nlp/index.js -> node_modules/rrule/dist/esm/src/nlp/totext.js -> node_modules/rrule/dist/esm/src/index.js
node_modules/rrule/dist/esm/src/index.js -> node_modules/rrule/dist/esm/src/rrule.js -> node_modules/rrule/dist/esm/src/nlp/index.js -> node_modules/rrule/dist/esm/src/nlp/parsetext.js -> node_modules/rrule/dist/esm/src/index.js
node_modules/rrule/dist/esm/src/index.js -> node_modules/rrule/dist/esm/src/rrule.js -> node_modules/rrule/dist/esm/src/nlp/index.js -> node_modules/rrule/dist/esm/src/index.js
...and 6 more

Thank you so much. I hope the MR is merged and released soon 🙂

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 a pull request may close this issue.

2 participants