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

Add support for default imports/exports #98

Closed
alangpierce opened this issue May 7, 2018 · 8 comments
Closed

Add support for default imports/exports #98

alangpierce opened this issue May 7, 2018 · 8 comments

Comments

@alangpierce
Copy link
Contributor

Simple example:

add.ts

export default function add(a: i32, b: i32): i32 {
  return a + b;
}

main.ts

import add from './add';

export function getValue(): i32 {
  return add(40, 2);
}

Currently both default import and default export syntax are a syntax error. In JS, this is mostly just syntax sugar over a named export called "default", so I think the expected behavior is reasonably straightforward.

I can try taking this on. Some initial thoughts on the steps/details:

  • Add support for import {default as foo} from './foo'; and export {foo as default}; so the other syntaxes can be tested individually.
  • Extend ImportStatement to have a default-imported name and make it behave like import {default as foo}.
  • Add a new CommonFlags.EXPORT_DEFAULT and mark it on export default function and export default class statements. I think these are technically live bindings for the class/function name, if specified, but maybe AssemblyScript should/does disallow overwriting classes and functions anyway?
  • Add a new statement type ExportDefaultStatement for export default [expression];. Hopefully this can be implemented similarly to export const.

Does that sound reasonable?

@dcodeIO
Copy link
Member

dcodeIO commented May 7, 2018

I think implicitly naming default imports default might be sufficient already. Not sure if additional flags or AST nodes are necessary. Should be just the parser that needs to understand a default following an export respectively an import without braces, somewhere in Parser#parseTopLevelStatement maybe.

alangpierce added a commit to alangpierce/assemblyscript that referenced this issue May 12, 2018
Progress toward AssemblyScript#98

See the imports and exports grammar from the spec:
https://www.ecma-international.org/ecma-262/8.0/index.html#sec-imports
https://www.ecma-international.org/ecma-262/8.0/index.html#sec-exports

In particular, ImportSpecifier and ExportSpecifier use IdentifierName to
identify the import/export name rather than the more restrictive ImportedBinding
(or just Identifier). That means any keyword is allowed.

There are (at least) three contexts to think about in JS identifier parsing:
* In most contexts, identifiers like variable names can be any name except a
  reserved word.
* Some keywords, like "as", are normally not keywords but have special
  keyword-like meaning in specific situations. Acorn and Babel call these
  "contextual keywords", and `tokenIsAlsoIdentifier` and `preferIdentifier` in
  AssemblyScript appear to handle that case.
* In some contexts, like object keys and named imports/exports, any name is
  allowed, even keywords.

This change adds that third case in order to handle imports/exports. I added a
`forceIdentifier` flag that tells the tokenizer that any identifier name should
be treated as a plain identifier, then used it in the import and export cases.
@alangpierce
Copy link
Contributor Author

Hmm, I worry about dropping the distinction at the parser level. One concern is just error messages and other tooling-related things; I've definitely seen custom error messages referencing default imports/exports, and I know CoffeeScript tooling is notoriously hard to write because so much information is dropped by the lexer/parser.

Also worth noting that every AST format in https://astexplorer.net/ (as far as I can tell) has a special case for default imports/exports, so it certainly seems common.

A few other cases:

export default function add() {} creates a name add that can be used in the file, so I think the AST needs to both keep the name add and note that it's a default export.

The syntax import a, * as b from 'c'; would be strange to parse and format using ASTBuilder (like in the parser tests) if a turned into {default as a}, since there isn't an import syntax that allows both star and named imports.

export default [expression]; is sort of its own thing in that it doesn't create an assignable binding, although I guess it's similar to a hypothetical export const default = [expression];, but I worry that there are maybe nuanced differences.

dcodeIO added a commit that referenced this issue Jun 4, 2019
Does not implement combinations like 'import theDefault, *' yet
dcodeIO added a commit that referenced this issue Jun 4, 2019
Does not implement combinations like 'import theDefault, *' yet
@dcodeIO
Copy link
Member

dcodeIO commented Jun 6, 2019

Quick update: There is some support for default exports and imports now, internally translating these to become a default member. Combined import syntax like import theDefault, * and import theDefault, { ... } is not yet supported.

@MaxGraey
Copy link
Member

Must be supported for quite some time

@MaxGraey MaxGraey reopened this Sep 11, 2019
@MaxGraey
Copy link
Member

Partially support. So stay opened

@andrewhassan
Copy link

I'm not sure if this ticket is related to the issue I'm having, but exporting the default exports from other files seems like it's not working within AssemblyScript.

In my case, I've created an index.ts file that exports the default exports from the files in its directory. I'm doing this using the following syntax:

export { default as Product } from './Product';

When I try to compile, I get the following error:

ERROR TS1003: Identifier expected.

 export { default as Product } from "./Product";
        ~
 in ~lib/index.ts(1,7)

As far as I can tell, these exports are valid, and the docs don't imply that this shouldn't work.

@MaxGraey
Copy link
Member

@dcodeIO It could be also closed I guess

@dcodeIO
Copy link
Member

dcodeIO commented May 16, 2020

Closing this issue as part of 2020 vacuum as it seems to have been fixed meanwhile, implemented as a default key internally, with the respective syntax mapping to it. If someone knows of or finds additional edge cases, I suggest to make a new issue with updated code samples so we know exactly what to look into.

@dcodeIO dcodeIO closed this as completed May 16, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants