guybedford / es-module-lexer

Low-overhead lexer dedicated to ES module parsing for fast analysis
MIT License
930 stars 48 forks source link

the `se` of dynamic import is always 0 #53

Closed csr632 closed 2 years ago

csr632 commented 4 years ago

Test code:

const { init, parse } = require("es-module-lexer");
(async () => {
  // either await init, or call parse asynchronously
  // this is necessary for the Web Assembly boot
  await init;

  const [imports, exports] = parse(`
  function loadDynamic() {
    import('react-dom').then(({ render }) => {
      dynamicImport.value = isFunction(render) ? 'success' : 'fail'
    })
  }
  `);
  console.log(imports);
})();

Output:

[ { s: 39, e: 50, ss: 32, se: 0, d: 32 } ]
guybedford commented 4 years ago

@csr632 yes dynamic import is an expression not a statement so this value holds no meaning. Instead use the usual start and end.

To be consistent we could just match it up to the same as the e value I suppose.

csr632 commented 4 years ago

@guybedford My use case is wraping the import promise. For example, I want to replace import('react') with import('react').then(m => m.default). So se would be very helpful.

guybedford commented 4 years ago

@csr632 the e value should be the closing ) of the expression though I think.

csr632 commented 4 years ago

I am confused with e and se after seeing your comment :) I expected source.substring(s, e) to be the specifier. And source.substring(ss, se) to be the whole expression import("react")

This is the @types/es-module-lexer :

// Type definitions for es-module-lexer 0.3
// Project: https://github.com/guybedford/es-module-lexer
// Definitions by: Tiger Oakes <https://github.com/NotWoods>
// Definitions: https://github.com/DefinitelyTyped/DefinitelyTyped

export interface ImportSpecifier {
    /**
     * Start of module specifier
     * @example
     * const source = `import { a } from 'asdf'`;
     * const [imports, exports] = parse(source);
     * source.substring(imports[0].s, imports[0].e);
     * // Returns "asdf"
     */
    s: number;
    /**
     * End of module specifier
     */
    e: number;

    /**
     * Start of import statement
     * @example
     * const source = `import { a } from 'asdf'`;
     * const [imports, exports] = parse(source);
     * source.substring(imports[0].s, imports[0].e);
     * // Returns "import { a } from 'asdf';"
     */
    ss: number;
    /**
     * End of import statement
     */
    se: number;

    /**
     * If this import statement is a dynamic import, this is the start value.
     * Otherwise this is `-1`.
     */
    d: number;
}

Does that match up with your intention?

guybedford commented 4 years ago

For dynamic import the start and end are the expression start and end, including the brackets I believe.

This is because you could have any custom expression in there, so we don't match the quotes or strings etc.

csr632 commented 4 years ago

That would solve my problem too! Thanks. Could you update the README to explain the meaning of s e ss se after implement this feature?

guybedford commented 4 years ago

It's already implemented - that's what it is.

guybedford commented 4 years ago

If you'd like to make a PR to clarify the docs that would be great. Or to set se to something sensible too. I'm open to suggestions. But the information is there already today is the point.

guybedford commented 2 years ago

The 0.10 release includes se always being defined for dynamic imports now, with ss always being the start as well, as suggested here, implemented in https://github.com/guybedford/es-module-lexer/pull/102.