Add support for importing reserved JS words

[mvlabat]

This should fix #1006

Implemented the idea explained in #1006 (comment).

Now

#[wasm_bindgen(module = "./SomeModule")]
extern "C" {
    #[wasm_bindgen(js_name = default)]
    type SomeType;
}

should be glued with

import { default as default1 } from './SomeModule';

I tested this behaviour on my local example project and it seems to be OK, though I couldn't get it working for the tests and import_js example. For some reason js_name attribute for types is just ignored there.

[mvlabat]

Oh, I see... I've forgotten to put js_class attribute for RenamedTypes functions. Though setting js_class = default doesn't seem to do anything either. Still digging...

Update. Forgot to recompile the target before running wasm-bindgen, lol. Works now :)

[mvlabat]

@Pauan, so I think we want to add everything from this list of global objects, don't we? We should be consistent with adding such things as undefined, arguments, eval.. Pretty much everything can break the code if a user decides to use such names in imports.

Update. Oh, won't do, it breaks everything global we generate bindings for :) Added only the value properties and arguments.

[Pauan]

@mvlabat arguments and eval are reserved words in strict mode:

https://tc39.github.io/ecma262/#sec-identifiers-static-semantics-early-errors

You can verify this by attempting to use the following JS code:

import { arguments, eval } from "foo";

You will get a SyntaxError.

As for globals (including undefined), we should not mangle them.

[mvlabat]

@Pauan alright, got it. And fixed

[mvlabat]

@Pauan Ahh.. And it seems like we need some kind of special treatment for eval. Like mangling it only for actual import statements. I'm not sure what's the better way to workaround this. Do you have any suggestions?

[Pauan]

@mvlabat I think the logic should be something like "if this identifier was imported, then mangle it, otherwise don't". It would need to run on every identifier, not just the imported identifiers.

[mvlabat]

@Pauan But it has to be mangled only on Import::Module, doesn't it? If it's not imported like that, then we don't need to mangle it on the first occurrence, because we do not declare a new variable, function or something like that conflicting with a reserved word. In case of Import::VendorPrefixed we already create a prefixed variable, in case of Import::Global we don't do anything at all and can use the name as is.

I'm not sure I understand this case correctly... If I'm mistaken, could you please explain it with an example? I'll try to reproduce it and come up with a fix then.

[Pauan]

@mvlabat Consider this JS code:

import { eval as eval1 } from "foo";

console.log(eval1);

As you can see, it needs to mangle it at both the import site and also the use site. Otherwise the use site will still be using eval, which is wrong.

Though I guess your RenamedTypes test probably accounts for that, so maybe it's already fine as-is?

[mvlabat]

@Pauan Oh, you meant this case. Yeah, it must work correctly. If I understand it right, generate_identifier isn't even supposed to be called twice in that case. If it did, we would actually have something like that:

import { eval as eval1 } from "foo";

console.log(eval2);

And there's nothing my code changed in this respect.

[mvlabat]

@alexcrichton, if everything looks ok, can we merge the PR and make a new release? ) If not, I'll surely try to fix it. Still can't use wasm-bindgen built locally with Parcel, and further development of my project is currently blocked without this change. Thank you!

[alexcrichton]

Thanks for the PR @mvlabat!

Instead of using lazy_static could this initialize a map per-Context and use that instead? Additionally how come only the import-from-a-module use case is handled here? Can these identifiers actually be names of functions in the global environment?

It may also be somewhat overkill to try to add an exhaustive list of keywords here, I suspect basically nothing comes up in practice except "default"?

[Pauan]

Can these identifiers actually be names of functions in the global environment?

Not under normal circumstances, since they're reserved words.

The only way to create/access them on the global environment is to use window.default (and similarly for the other names).

[mvlabat]

@alexcrichton

Instead of using lazy_static could this initialize a map per-Context and use that instead?

I think that lazy_static is a more preferable choice here, because we don't need to reinitialize the set of the keywords: it's not the map that's being mutated, it's really just a static set. Creating it for each context seems to be redundant.

Additionally how come only the import-from-a-module use case is handled here? Can these identifiers actually be names of functions in the global environment?

If we want to support this, it's possible, I think. Though it would be a really, really rare case. I'm just not sure if we can return right from generate_identifier something like this: window.identifier (for browsers) or global.identifier (for node.js).

It may also be somewhat overkill to try to add an exhaustive list of keywords here, I suspect basically nothing comes up in practice except "default"?

If we want to support only default exports, then I can publish an alternative PR with much fewer lines of code, that handles only that specific case. And I think all the questions above will just become irrelevant )

[alexcrichton]

Ok thanks for the info! I'd be curious to see if we could just handle the default case here yeah and see if that'll get us basically 99% of the way there

[alexcrichton]

Closing in favor of #1106, thanks again @mvlabat!