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

Tape does not pass require.extensions to resolve #395

Closed
snuggs opened this issue Sep 22, 2017 · 11 comments · Fixed by #396
Closed

Tape does not pass require.extensions to resolve #395

snuggs opened this issue Sep 22, 2017 · 11 comments · Fixed by #396

Comments

@snuggs
Copy link
Contributor

snuggs commented Sep 22, 2017

We are currently using the legacy .es ECMAScript file extension. How we are using this is by appending a .es key to require.extensions. Have discussed with members of node and this has been converted into a public API since ffa503a which introduced resolve into tape. However, resolve will not use the default require .extensions see https://github.com/browserify/resolve/blob/5f6e682396eca043624d0a13286b9015b1e6cfce/lib/sync.js#L23 . Already have a commit that merely adds { extensions: require.extensions } to the call to resolve. Please let me know if I should be opening an issue on tape (preferred) or in resolve (possibly should do this as well). Since they have a mechanism to change extensions I feel will be a great start here. Will notify resolve team as well. At the current state have no way to alter resolve and no way to alter what tape sends to resolve. Thanks in advance!

Thanks in advance. /cc @brandondees

capture d ecran 2017-09-21 a 17 34 32

@snuggs
Copy link
Contributor Author

snuggs commented Sep 22, 2017

Also noticed -r has the same semantics as this issue addresses. my polyfill file is where the require.extensions swizzle occurs. Hence why my test.es file never loads from -r and --require

capture d ecran 2017-09-21 a 18 39 30

@ljharb
Copy link
Collaborator

ljharb commented Sep 22, 2017

.es has never been a standard nor a recommended file extension; it's not legacy, it's just an arbitrary thing you chose - the same as if you'd used .yogurt.

I think I agree that it makes sense for tape to provide require.extensions to resolve; I'll review your PR.

@ljharb
Copy link
Collaborator

ljharb commented Sep 22, 2017

See #396 (comment) for why this isn't the proper fix.

Perhaps an --extensions option to tape (that is passed to resolve) would address your issue?

@snuggs
Copy link
Contributor Author

snuggs commented Sep 22, 2017

@ljharb I understand this is common misconception. But the facts are this has not only been considered as a file extension but speced and thoroughly documented in [IETF rfc4329 _Section 8.2_](https://tools.ietf.org/html/rfc4329#section-8.2).

I've also taken the liberty to document this on MDN and ad nauseam in the snuggsiツ library
https://github.com/devpunks/snuggsi/tree/master/dist#readme

Also has long been the standard extension as far as Wikipedia is concerned. Do you believe we should update the docs? Am open if you suggest but it will be a long journey, trust.
https://en.wikipedia.org/wiki/ECMAScript

capture d ecran 2017-09-22 a 12 21 48

Can also link you to a lengthy deliberation on the matter with the W3C and WHATWG. Please trust application/ecmascript is real. Within the context of application/javascript your stance is 💯 valid to be clear.

I trust you'd understand as someone who is a prolific contributor to OSS that I would not waste your time with .yogurt. I also feel this is superfluous to the request. That being said, as you recognized in your second comment, anything would do and i'd be extremely pleased. I just want to get my work done (don't we all). The fact you are even considering this makes me feel great. And i would never suggest something I wasn't willing to contribute to myself given the consensus.

Please advise.

@ljharb
Copy link
Collaborator

ljharb commented Sep 22, 2017

I'm aware that .es is registered, but no implementations have ever supported it, nor do any plan to, nor has any significant percentage of the community adopted it. node is going with .mjs for ES Modules, and some frameworks have adopted a few (like .jsx, .vue, etc) - but that's about it.

It's superfluous to the request, but I didn't want future readers to assume that .es was ever anything more than an overly optimistic IETF registration.

@snuggs
Copy link
Contributor Author

snuggs commented Sep 22, 2017

@ljharb Totally! And thanks for making me aware of that. Our future selves are just as important to take into consideration. I'm actually documenting this over at the WHATWG. https://github.com/whatwg/meta/pull/40/files#diff-16d8907348d1a44d1fe4803d38be499eR33

@snuggs
Copy link
Contributor Author

snuggs commented Mar 15, 2018

It's superfluous to the request, but I didn't want future readers to assume that .es was ever anything more than an overly optimistic IETF registration @ljharb

Well... text/javascript and application/javascript defined in the very spec you speak of were also included with application/ecmascript. All 3 by the way are 100% valid HTML Javascript MimeTypes. What we call the extension is superfluous. As it should be. Just ask .mjs. But in certain ecosystems it's not.

We've been here before](https://mail.mozilla.org/pipermail/es-discuss/2011-August/016267.html) I'd be curious what Brendan and Douglas have to say about "optimisim". They were one of the first pumping the "optimism" with application/ecmascript & application/javascript. Some Fortune 500 companies (that use this library) have to deal with developer decisions from a decade ago.

I either have to tell them they can use tape or not. If i tell them to not use it I'm the one that recommended it. It's really that simple for me. I'm a loyal user. They could care less. If it doesn't support legacy or do what it says it's going to do I have to report this.

Hope this clears things up. Also clears up in retrospect this may be the wrong library to make these statements.

@ljharb
Copy link
Collaborator

ljharb commented Mar 15, 2018

Here are the choices that can be made for tape:

  1. no change; everybody loses
  2. Tape does not pass require.extensions to resolve #395 (comment) - add an --extensions option, including some way to indicate "just use require.extensions dynamically". Then, you (and users like you) can do something like tape --require path/to/extensions/polyfill --extensions paths/to/files and everything works, without breaking any users of the tape binary.
  3. Make the breaking change of changing tape's default to be "pass require.extensions"

My preference is option 2 for now, and if the common use case becomes "always pass require.extensions", then option 3 would be sufficiently motivated.

@snuggs, does option 2 seem like it would address your use case?

@snuggs
Copy link
Contributor Author

snuggs commented Mar 15, 2018

@ljharb hey man just here to help :-) Whatever keeps us moving forward and in the game! I just know if it's going to come up for me it will come up for some (few) others. Best to be aware sooner than later. The fact you are even considering to do something says a lot. I'm open for ANYTHING as we have nothing currently. YES this IS a "leaky abstraction" of underlying implementation details with Tape. But not like we are forcing people to use the flag anyways. May be our best bet.

Thoughts?

@ljharb
Copy link
Collaborator

ljharb commented Mar 15, 2018

Great - I'll think about how to implement this, so at least there's a path forward without a breaking change.

@snuggs
Copy link
Contributor Author

snuggs commented Mar 15, 2018

@ljharb i'm on board...and here to help. As a maintainer myself I understand the pain when someone requests a breaking change.

P.S. Thank you tape for helping us kick butt with async tests :-) #homage

devpunks/snuggsi#164 (comment)

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