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

compat: xregexp doesn't work in compat mode #13424

Closed
bartlomieju opened this issue Jan 19, 2022 · 4 comments · Fixed by #15678
Closed

compat: xregexp doesn't work in compat mode #13424

bartlomieju opened this issue Jan 19, 2022 · 4 comments · Fixed by #15678
Assignees
Labels
bug Something isn't working correctly node compat

Comments

@bartlomieju
Copy link
Member

bartlomieju commented Jan 19, 2022

@lucacasonato @bartlomieju

I just did the test based on:

npm i xregexp 
deno run --allow-all  --unstable --compat importTest.js

importTest.js file is:

// @deno-types='./node_modules/xregexp/types/index.d.ts'
import XRegExp from  './node_modules/xregexp/src/index.js'

The source index.js file looks like this:

import XRegExp from './xregexp';

import build from './addons/build';
import matchRecursive from './addons/matchrecursive';
import unicodeBase from './addons/unicode-base';
import unicodeCategories from './addons/unicode-categories';
import unicodeProperties from './addons/unicode-properties';
import unicodeScripts from './addons/unicode-scripts';

build(XRegExp);
matchRecursive(XRegExp);
unicodeBase(XRegExp);
unicodeCategories(XRegExp);
unicodeProperties(XRegExp);
unicodeScripts(XRegExp);

export default XRegExp;

And the package.json file includes the line:

 module: "./src/index.js"

I tried different combinations, renaming the test file to importTest.mjs or setting "type": "module" in my own package.json, or fetching the index.js from /src/index.js instead of /lib. but I get various missing module, no default export errors.

I am not an expert in ESM, but as stated earlier, the code is pretty much vanilla JavaScript and I know what to change to make it work. in the Deno environment.

I do not know the specifics of how Node do or do not support 'magic resolution of imports' for ESM and/or vanilla JavaScript, but keep in mind, that this is working code using 'import' within the Nodejs environment with 7,890,538 million weekly downloads.

Originally posted by @cfjello in #2506 (comment)

@bartlomieju
Copy link
Member Author

bartlomieju commented Jan 19, 2022

// test.js
// @deno-types='./node_modules/xregexp/types/index.d.ts'
import XRegExp from './node_modules/xregexp/src/index.js'
// package.json
{
  "type": "module",
  "dependencies": {
    "xregexp": "^5.1.0"
  }
}
$ deno run -A --unstable --compat test.js
error: [ERR_MODULE_NOT_FOUND] Cannot find module "file:///Users/ib/dev/xregexp/node_modules/xregexp/src/addons/build" imported from "file:///Users/ib/dev/xregexp/node_modules/xregexp/src/index.js"
    at file:///Users/ib/dev/xregexp/node_modules/xregexp/src/index.js:3:19

@bartlomieju bartlomieju added node compat bug Something isn't working correctly labels Jan 19, 2022
@bartlomieju bartlomieju self-assigned this Jan 19, 2022
@cfjello
Copy link

cfjello commented Jan 20, 2022

@bartlomieju
Thanks! It is great that you are taking a look at this!

Bear with me if I am stating the obvious, but maybe this could be helpful:

  • The imports with missing file extensions can be found all through the package, not only in the top level index.js file.
  • Some files, like the tools/output/categories.js file, imported in src/addons/unicode-categories.js, only contains an unnamed object of the type module.exports = [ {....}], meaning that the name of the variable/module must be inferred from the file name.

Let me know, if I can help with anything :-)

@bartlomieju
Copy link
Member Author

Some files, like the tools/output/categories.js file, imported in src/addons/unicode-categories.js, only contains an unnamed object of the type module.exports = [ {....}], meaning that the name of the variable/module must be inferred from the file name.

@cfjello right, so this is the crux of the problem - since these files are CJS (because of module.exports syntax) when they are imported from ES module, they need to be translated on the fly into ES modules too. This is already covered in #12648 and I'm actively working on a fix.

@bartlomieju
Copy link
Member Author

Current state:

cargo run -- run -A --unstable --compat test.js
    Finished dev [unoptimized + debuginfo] target(s) in 0.17s
     Running `target/debug/deno run -A --unstable --compat test.js`
error: 'import', and 'export' cannot be used outside of module code at file:///Users/ib/dev/deno/node_modules/xregexp/src/index.js:1:1

It seems something is still of - there's module entry in package.json but Deno doesn't recognize that it's an ESM file and considers it as CommonJs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly node compat
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants